Skip to content

Commit

Permalink
fix(ast, codegen, linter): panics in fixers. (#5431)
Browse files Browse the repository at this point in the history
  • Loading branch information
rzvxa committed Sep 5, 2024
1 parent 9f6e0ed commit 0df1d9d
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 19 deletions.
8 changes: 5 additions & 3 deletions crates/oxc_ast/src/ast_impl/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use crate::ast::*;

use std::{
borrow::Cow,
fmt,
hash::{Hash, Hasher},
};
Expand Down Expand Up @@ -132,10 +133,11 @@ impl<'a> RegExpPattern<'a> {
self.len() == 0
}

pub fn source_text(&self, source_text: &'a str) -> &'a str {
pub fn source_text(&self, source_text: &'a str) -> Cow<str> {
match self {
Self::Raw(raw) | Self::Invalid(raw) => raw,
Self::Pattern(pat) => pat.span.source_text(source_text),
Self::Raw(raw) | Self::Invalid(raw) => Cow::Borrowed(raw),
Self::Pattern(pat) if pat.span.is_unspanned() => Cow::Owned(pat.to_string()),
Self::Pattern(pat) => Cow::Borrowed(pat.span.source_text(source_text)),
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_codegen/src/annotation_comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,22 @@ impl<'a> Codegen<'a> {
return vec![];
}
let mut latest_comment_start = node_start;
let source_text = self.source_text.unwrap_or_default();
let mut ret = self
.get_leading_comments(self.latest_consumed_comment_end, node_start)
.rev()
// each comment should be separated by whitespaces
.take_while(|comment| {
let comment_end = comment.real_span_end();
let range_content =
&self.source_text[comment_end as usize..latest_comment_start as usize];
&source_text[comment_end as usize..latest_comment_start as usize];
let all_whitespace = range_content.chars().all(char::is_whitespace);
latest_comment_start = comment.real_span_start();
all_whitespace
})
.filter_map(|comment| {
let source_code = self.source_text;
let comment_content =
&source_code[comment.span.start as usize..comment.span.end as usize];
&source_text[comment.span.start as usize..comment.span.end as usize];
if let Some(m) = MATCHER.find_iter(&comment_content).next() {
let annotation_kind = match m.value() {
0 | 1 => AnnotationKind::NO_SIDE_EFFECTS,
Expand Down
9 changes: 6 additions & 3 deletions crates/oxc_codegen/src/gen.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::ops::Not;
use std::{borrow::Cow, ops::Not};

use oxc_allocator::{Box, Vec};
#[allow(clippy::wildcard_imports)]
Expand Down Expand Up @@ -1234,15 +1234,18 @@ impl<'a> Gen for RegExpLiteral<'a> {
fn gen(&self, p: &mut Codegen, _ctx: Context) {
p.add_source_mapping(self.span.start);
let last = p.peek_nth(0);
let pattern_text = self.regex.pattern.source_text(p.source_text);
let pattern_text = p.source_text.map_or_else(
|| Cow::Owned(self.regex.pattern.to_string()),
|src| self.regex.pattern.source_text(src),
);
// Avoid forming a single-line comment or "</script" sequence
if Some('/') == last
|| (Some('<') == last && pattern_text.to_lowercase().starts_with("script"))
{
p.print_hard_space();
}
p.print_char(b'/');
p.print_str(pattern_text);
p.print_str(pattern_text.as_ref());
p.print_char(b'/');
p.print_str(self.regex.flags.to_string().as_str());
p.prev_reg_exp_end = p.code().len();
Expand Down
23 changes: 18 additions & 5 deletions crates/oxc_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ pub struct Codegen<'a> {
options: CodegenOptions,
comment_options: CommentOptions,

source_text: &'a str,
/// Original source code of the AST
source_text: Option<&'a str>,

trivias: Trivias,

Expand Down Expand Up @@ -131,7 +132,7 @@ impl<'a> Codegen<'a> {
Self {
options: CodegenOptions::default(),
comment_options: CommentOptions::default(),
source_text: "",
source_text: None,
trivias: Trivias::default(),
mangler: None,
code: vec![],
Expand Down Expand Up @@ -169,17 +170,25 @@ impl<'a> Codegen<'a> {
self
}

/// Adds the source text of the original AST, It is used with comments or for improving the
/// generated output.
#[must_use]
pub fn with_source_text(mut self, source_text: &'a str) -> Self {
self.source_text = Some(source_text);
self
}

/// Also sets the [Self::with_source_text]
#[must_use]
pub fn enable_comment(
mut self,
source_text: &'a str,
trivias: Trivias,
options: CommentOptions,
) -> Self {
self.source_text = source_text;
self.trivias = trivias;
self.comment_options = options;
self
self.with_source_text(source_text)
}

#[must_use]
Expand Down Expand Up @@ -539,8 +548,12 @@ impl<'a> Codegen<'a> {
/// Avoid issue related to rustc borrow checker .
/// Since if you want to print a range of source code, you need to borrow the source code
/// as immutable first, and call the [Self::print_str] which is a mutable borrow.
///
/// # Panics
/// If `self.source_text` isn't set.
fn print_range_of_source_code(&mut self, range: Range<usize>) {
self.code.extend_from_slice(self.source_text[range].as_bytes());
let source_text = self.source_text.expect("expect `Codegen::source_text` to be set.");
self.code.extend_from_slice(source_text[range].as_bytes());
}

fn get_leading_comments(
Expand Down
11 changes: 11 additions & 0 deletions crates/oxc_codegen/tests/integration/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ pub fn test(source_text: &str, expected: &str) {
);
}

pub fn test_without_source(source_text: &str, expected: &str) {
let source_type = SourceType::default().with_module(true).with_jsx(true);
let allocator = Allocator::default();
let ret = Parser::new(&allocator, source_text, source_type).parse();
let result = CodeGenerator::new().build(&ret.program).source_text;
assert_eq!(
result, expected,
"\nfor source {source_text:?}\nexpect {expected:?}\ngot {result:?}\nwithout providing the original code."
);
}

pub fn test_minify(source_text: &str, expected: &str) {
let source_type = SourceType::default().with_module(true).with_jsx(true);
let allocator = Allocator::default();
Expand Down
29 changes: 28 additions & 1 deletion crates/oxc_codegen/tests/integration/unit.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::tester::{test, test_minify};
use crate::tester::{test, test_minify, test_without_source};

#[test]
fn module_decl() {
Expand Down Expand Up @@ -63,6 +63,33 @@ fn unicode_escape() {
test("console.log('🧑‍🤝‍🧑');", "console.log(\"🧑‍🤝‍🧑\");\n");
}

#[test]
fn regex() {
fn test_all(source: &str, expect: &str, minify: &str) {
test(source, expect);
test_minify(source, minify);
test_without_source(source, expect);
}
test_all("/regex/giv", "/regex/giv;\n", "/regex/giv;");
test_all(
r"/(.)(.)(.)(.)(.)(.)(.)(.)\8\8/",
"/(.)(.)(.)(.)(.)(.)(.)(.)\\8\\8/;\n",
"/(.)(.)(.)(.)(.)(.)(.)(.)\\8\\8/;",
);

test_all(
r"/\n\cM\0\x41\u{1f600}\./u",
"/\\n\\cM\\0\\x41\\u{1f600}\\./u;\n",
"/\\n\\cM\\0\\x41\\u{1f600}\\./u;",
);
test_all(r"/\n\cM\0\x41\./u", "/\\n\\cM\\0\\x41\\./u;\n", "/\\n\\cM\\0\\x41\\./u;");
test_all(
r"/\n\cM\0\x41\u1234\./",
"/\\n\\cM\\0\\x41\\u1234\\./;\n",
"/\\n\\cM\\0\\x41\\u1234\\./;",
);
}

#[test]
fn comma() {
test_minify("1, 2, 3", "1,2,3;");
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_linter/src/fixer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ impl<'c, 'a: 'c> RuleFixer<'c, 'a> {
#[allow(clippy::unused_self)]
pub fn codegen(self) -> CodeGenerator<'a> {
CodeGenerator::new()
.with_source_text(self.source_text())
.with_options(CodegenOptions { single_quote: true, ..CodegenOptions::default() })
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_control_regex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl Rule for NoControlRegex {
let mut violations: Vec<&str> = Vec::new();
let pattern = pattern.as_ref();
let pattern_text = pattern.source_text(context.source_text());
for matched_ctl_pattern in control_patterns(pattern_text) {
for matched_ctl_pattern in control_patterns(pattern_text.as_ref()) {
let ctl = matched_ctl_pattern.as_str();

// check for an even number of backslashes, since these will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl Rule for NoEmptyCharacterClass {

if let AstKind::RegExpLiteral(lit) = node.kind() {
if !NO_EMPTY_CLASS_REGEX_PATTERN
.is_match(lit.regex.pattern.source_text(ctx.source_text()))
.is_match(lit.regex.pattern.source_text(ctx.source_text()).as_ref())
{
ctx.diagnostic(no_empty_character_class_diagnostic(lit.span));
}
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_linter/src/rules/eslint/no_regex_spaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ impl Rule for NoRegexSpaces {
impl NoRegexSpaces {
fn find_literal_to_report(literal: &RegExpLiteral, ctx: &LintContext) -> Option<Span> {
let pattern_text = literal.regex.pattern.source_text(ctx.source_text());
let pattern_text = pattern_text.as_ref();
if Self::has_exempted_char_class(pattern_text) {
return None;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/unicorn/no_hex_escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl Rule for NoHexEscape {
}
AstKind::RegExpLiteral(regex) => {
if let Some(fixed) =
check_escape(regex.regex.pattern.source_text(ctx.source_text()))
check_escape(regex.regex.pattern.source_text(ctx.source_text()).as_ref())
{
#[allow(clippy::cast_possible_truncation)]
ctx.diagnostic_with_fix(no_hex_escape_diagnostic(regex.span), |fixer| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ fn get_pattern_replacement<'a>(
}

let pattern_text = reg_exp_literal.regex.pattern.source_text(ctx.source_text());
let pattern_text = pattern_text.as_ref();
if !is_simple_string(pattern_text) {
return None;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ impl Rule for PreferStringStartsEndsWith {
};

let pattern_text = regex.regex.pattern.source_text(ctx.source_text());
let pattern_text = pattern_text.as_ref();

let Some(err_kind) = check_regex(regex, pattern_text) else {
return;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_prettier/src/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1416,7 +1416,7 @@ impl<'a> Format<'a> for RegExpLiteral<'a> {
fn format(&self, p: &mut Prettier<'a>) -> Doc<'a> {
let mut parts = p.vec();
parts.push(ss!("/"));
parts.push(p.str(self.regex.pattern.source_text(p.source_text)));
parts.push(p.str(self.regex.pattern.source_text(p.source_text).as_ref()));
parts.push(ss!("/"));
parts.push(format!(p, self.regex.flags));
Doc::Array(parts)
Expand Down

0 comments on commit 0df1d9d

Please sign in to comment.