Skip to content

Commit

Permalink
feat(ast)!: change comment.span to real position that contain //
Browse files Browse the repository at this point in the history
…and `/*` (#7154)

closes #7150
  • Loading branch information
Boshen committed Nov 6, 2024
1 parent 19892ed commit d1d1874
Show file tree
Hide file tree
Showing 27 changed files with 132 additions and 168 deletions.
35 changes: 13 additions & 22 deletions crates/oxc_ast/src/ast/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub enum CommentPosition {
#[generate_derive(CloneIn, ContentEq, ContentHash)]
#[derive(Debug, Default, Clone, Copy, Eq, PartialEq)]
pub struct Comment {
/// The span of the comment text (without leading/trailing delimiters).
/// The span of the comment text, with leading and trailing delimiters.
pub span: Span,

/// Start of token this leading comment is attached to.
Expand Down Expand Up @@ -101,29 +101,12 @@ impl Comment {
self.position == CommentPosition::Trailing
}

#[allow(missing_docs)]
pub fn real_span(&self) -> Span {
Span::new(self.real_span_start(), self.real_span_end())
}

#[allow(missing_docs)]
pub fn real_span_end(&self) -> u32 {
match self.kind {
CommentKind::Line => self.span.end,
// length of `*/`
CommentKind::Block => self.span.end + 2,
}
}

#[allow(missing_docs)]
pub fn real_span_start(&self) -> u32 {
self.span.start - 2
}

/// Returns `true` if this comment is a JSDoc comment. Implies `is_leading`
/// and `is_block`.
pub fn is_jsdoc(&self, source_text: &str) -> bool {
self.is_leading() && self.is_block() && self.span.source_text(source_text).starts_with('*')
self.is_leading()
&& self.is_block()
&& self.content_span().source_text(source_text).starts_with('*')
}

/// Legal comments
Expand All @@ -135,9 +118,17 @@ impl Comment {
if !self.is_leading() {
return false;
}
let source_text = self.span.source_text(source_text);
let source_text = self.content_span().source_text(source_text);
source_text.starts_with('!')
|| source_text.contains("@license")
|| source_text.contains("@preserve")
}

/// Gets the span of the comment content.
pub fn content_span(&self) -> Span {
match self.kind {
CommentKind::Line => Span::new(self.span.start + 2, self.span.end),
CommentKind::Block => Span::new(self.span.start + 2, self.span.end - 2),
}
}
}
10 changes: 5 additions & 5 deletions crates/oxc_codegen/src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl<'a> Codegen<'a> {
///
/// <https://github.com/javascript-compiler-hints/compiler-notations-spec/blob/main/pure-notation-spec.md>
fn is_annotation_comment(&self, comment: &Comment) -> bool {
let s = comment.span.source_text(self.source_text).trim_start();
let s = comment.content_span().source_text(self.source_text).trim_start();
if let Some(s) = s.strip_prefix(['@', '#']) {
s.starts_with("__PURE__") || s.starts_with("__NO_SIDE_EFFECTS__")
} else {
Expand All @@ -54,7 +54,7 @@ impl<'a> Codegen<'a> {
comment.preceded_by_newline
&& (comment.is_jsdoc(self.source_text)
|| (comment.is_line() && self.is_annotation_comment(comment)))
&& !comment.span.source_text(self.source_text).chars().all(|c| c == '*')
&& !comment.content_span().source_text(self.source_text).chars().all(|c| c == '*')
// webpack comment `/*****/`
}

Expand Down Expand Up @@ -126,10 +126,10 @@ impl<'a> Codegen<'a> {
}
if comment.is_line() {
self.print_str("/*");
self.print_str(comment.span.source_text(self.source_text));
self.print_str(comment.content_span().source_text(self.source_text));
self.print_str("*/");
} else {
self.print_str(comment.real_span().source_text(self.source_text));
self.print_str(comment.span.source_text(self.source_text));
}
self.print_hard_space();
}
Expand Down Expand Up @@ -205,7 +205,7 @@ impl<'a> Codegen<'a> {
}

fn print_comment(&mut self, comment: &Comment) {
let comment_source = comment.real_span().source_text(self.source_text);
let comment_source = comment.span.source_text(self.source_text);
match comment.kind {
CommentKind::Line => {
self.print_str(comment_source);
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_codegen/tests/integration/legal_comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ fn legal_external_comment() {
let code = "/* @license */\n/* @preserve */\nfoo;\n";
let ret = codegen_options(code, &options);
assert_eq!(ret.code, "foo;\n");
assert_eq!(ret.legal_comments[0].span.source_text(code), " @license ");
assert_eq!(ret.legal_comments[1].span.source_text(code), " @preserve ");
assert_eq!(ret.legal_comments[0].content_span().source_text(code), " @license ");
assert_eq!(ret.legal_comments[1].content_span().source_text(code), " @preserve ");
}
3 changes: 2 additions & 1 deletion crates/oxc_isolated_declarations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ impl<'a> IsolatedDeclarations<'a> {
fn build_internal_annotations(program: &Program<'a>) -> FxHashSet<u32> {
let mut set = FxHashSet::default();
for comment in &program.comments {
let has_internal = comment.span.source_text(program.source_text).contains("@internal");
let has_internal =
comment.content_span().source_text(program.source_text).contains("@internal");
// Use the first jsdoc comment if there are multiple jsdoc comments for the same node.
if has_internal && !set.contains(&comment.attached_to) {
set.insert(comment.attached_to);
Expand Down
48 changes: 19 additions & 29 deletions crates/oxc_linter/src/disable_directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ impl<'a> DisableDirectivesBuilder<'a> {
// for matching disable and enable pairs.
// Wrongly ordered matching pairs are not taken into consideration.
for comment in comments {
let text = comment.span.source_text(source_text);
let span = comment.content_span();
let text = span.source_text(source_text);
let text = text.trim_start();

if let Some(text) =
Expand All @@ -100,59 +101,53 @@ impl<'a> DisableDirectivesBuilder<'a> {
// `eslint-disable`
if text.trim().is_empty() {
if self.disable_all_start.is_none() {
self.disable_all_start = Some(comment.span.end);
self.disable_all_start = Some(span.end);
}
self.disable_all_comments.push(comment.span);
self.disable_all_comments.push(span);
continue;
}
// `eslint-disable-next-line`
else if let Some(text) = text.strip_prefix("-next-line") {
// Get the span up to the next new line
let stop = source_text[comment.span.end as usize..]
let stop = source_text[span.end as usize..]
.lines()
.take(2)
.fold(comment.span.end, |acc, line| acc + line.len() as u32);
.fold(span.end, |acc, line| acc + line.len() as u32);
if text.trim().is_empty() {
self.add_interval(comment.span.end, stop, DisabledRule::All);
self.disable_all_comments.push(comment.span);
self.add_interval(span.end, stop, DisabledRule::All);
self.disable_all_comments.push(span);
} else {
// `eslint-disable-next-line rule_name1, rule_name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.add_interval(
comment.span.end,
stop,
DisabledRule::Single(rule_name),
);
self.add_interval(span.end, stop, DisabledRule::Single(rule_name));
rules.push(rule_name);
});
self.disable_rule_comments
.push(DisableRuleComment { span: comment.span, rules });
self.disable_rule_comments.push(DisableRuleComment { span, rules });
}
continue;
}
// `eslint-disable-line`
else if let Some(text) = text.strip_prefix("-line") {
// Get the span between the preceding newline to this comment
let start = source_text[..comment.span.start as usize]
let start = source_text[..span.start as usize]
.lines()
.next_back()
.map_or(0, |line| comment.span.start - line.len() as u32);
let stop = comment.span.start;
.map_or(0, |line| span.start - line.len() as u32);
let stop = span.start;

// `eslint-disable-line`
if text.trim().is_empty() {
self.add_interval(start, stop, DisabledRule::All);
self.disable_all_comments.push(comment.span);
self.disable_all_comments.push(span);
} else {
// `eslint-disable-line rule-name1, rule-name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.add_interval(start, stop, DisabledRule::Single(rule_name));
rules.push(rule_name);
});
self.disable_rule_comments
.push(DisableRuleComment { span: comment.span, rules });
self.disable_rule_comments.push(DisableRuleComment { span, rules });
}
continue;
}
Expand All @@ -162,11 +157,10 @@ impl<'a> DisableDirectivesBuilder<'a> {
// `eslint-disable rule-name1, rule-name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.disable_start_map.entry(rule_name).or_insert(comment.span.end);
self.disable_start_map.entry(rule_name).or_insert(span.end);
rules.push(rule_name);
});
self.disable_rule_comments
.push(DisableRuleComment { span: comment.span, rules });
self.disable_rule_comments.push(DisableRuleComment { span, rules });
continue;
}
}
Expand All @@ -177,17 +171,13 @@ impl<'a> DisableDirectivesBuilder<'a> {
// `eslint-enable`
if text.trim().is_empty() {
if let Some(start) = self.disable_all_start.take() {
self.add_interval(start, comment.span.start, DisabledRule::All);
self.add_interval(start, span.start, DisabledRule::All);
}
} else {
// `eslint-enable rule-name1, rule-name2`
Self::get_rule_names(text, |rule_name| {
if let Some(start) = self.disable_start_map.remove(rule_name) {
self.add_interval(
start,
comment.span.start,
DisabledRule::Single(rule_name),
);
self.add_interval(start, span.start, DisabledRule::Single(rule_name));
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/default_case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl Rule for DefaultCase {
.comments_range(last_case.span.start..switch.span.end)
.last()
.is_some_and(|comment| {
let raw = comment.span.source_text(ctx.semantic().source_text()).trim();
let raw = comment.content_span().source_text(ctx.source_text()).trim();
match &self.comment_pattern {
Some(comment_pattern) => comment_pattern.is_match(raw),
None => raw.eq_ignore_ascii_case("no default"),
Expand Down
11 changes: 6 additions & 5 deletions crates/oxc_linter/src/rules/eslint/max_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ impl Rule for MaxLines {
let comment_lines = if self.skip_comments {
let mut comment_lines: usize = 0;
for comment in ctx.semantic().comments() {
let comment_span = comment.content_span();
if comment.is_line() {
let comment_line = ctx.source_text()[..comment.span.start as usize]
let comment_line = ctx.source_text()[..comment_span.start as usize]
.lines()
.next_back()
.unwrap_or("");
Expand All @@ -93,18 +94,18 @@ impl Rule for MaxLines {
}
} else {
let mut start_line =
ctx.source_text()[..comment.span.start as usize].lines().count();
let comment_start_line = ctx.source_text()[..comment.span.start as usize]
ctx.source_text()[..comment_span.start as usize].lines().count();
let comment_start_line = ctx.source_text()[..comment_span.start as usize]
.lines()
.next_back()
.unwrap_or("");
if !line_has_just_comment(comment_start_line, "/*") {
start_line += 1;
}
let mut end_line =
ctx.source_text()[..=comment.span.end as usize].lines().count();
ctx.source_text()[..=comment_span.end as usize].lines().count();
let comment_end_line =
ctx.source_text()[comment.span.end as usize..].lines().next().unwrap_or("");
ctx.source_text()[comment_span.end as usize..].lines().next().unwrap_or("");
if line_has_just_comment(comment_end_line, "*/") {
end_line += 1;
}
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_linter/src/rules/eslint/no_fallthrough.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ fn possible_fallthrough_comment_span(case: &SwitchCase) -> (u32, Option<u32>) {

impl NoFallthrough {
fn has_blanks_between(ctx: &LintContext, range: Range<u32>) -> bool {
let in_between = &ctx.semantic().source_text()[range.start as usize..range.end as usize];
let in_between = &ctx.source_text()[range.start as usize..range.end as usize];
// check for at least 2 new lines, we allow the first new line for formatting.
in_between.bytes().filter(|it| *it == b'\n').nth(1).is_some()
}
Expand All @@ -382,9 +382,7 @@ impl NoFallthrough {
let is_fallthrough_comment_in_range = |range: Range<u32>| {
let comment = semantic
.comments_range(range)
.map(|comment| {
&semantic.source_text()[comment.span.start as usize..comment.span.end as usize]
})
.map(|comment| comment.content_span().source_text(semantic.source_text()))
.last()
.map(str::trim);

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/sort_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl Rule for SortKeys {

let mut property_groups: Vec<Vec<String>> = vec![vec![]];

let source_text = ctx.semantic().source_text();
let source_text = ctx.source_text();

for (i, prop) in dec.properties.iter().enumerate() {
match prop {
Expand Down
7 changes: 3 additions & 4 deletions crates/oxc_linter/src/rules/jest/no_commented_out_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,15 @@ impl Rule for NoCommentedOutTests {
Regex::new(r#"(?mu)^\s*[xf]?(test|it|describe)(\.\w+|\[['"]\w+['"]\])?\s*\("#).unwrap();
}
let comments = ctx.semantic().comments();
let source_text = ctx.semantic().source_text();
let source_text = ctx.source_text();
let commented_tests = comments.iter().filter_map(|comment| {
let text = comment.span.source_text(source_text);
let text = comment.content_span().source_text(source_text);
if RE.is_match(text) {
Some(comment.span)
Some(comment.content_span())
} else {
None
}
});

for span in commented_tests {
ctx.diagnostic(no_commented_out_tests_diagnostic(span));
}
Expand Down
12 changes: 6 additions & 6 deletions crates/oxc_linter/src/rules/typescript/ban_ts_comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl Rule for BanTsComment {
fn run_once(&self, ctx: &LintContext) {
let comments = ctx.semantic().comments();
for comm in comments {
let raw = ctx.source_range(comm.span);
let raw = ctx.source_range(comm.content_span());
if let Some(captures) = find_ts_comment_directive(raw, comm.is_line()) {
// safe to unwrap, if capture success, it can always capture one of the four directives
let (directive, description) = (captures.0, captures.1);
Expand All @@ -178,16 +178,16 @@ impl Rule for BanTsComment {
if *on {
if directive == "ignore" {
ctx.diagnostic_with_fix(
ignore_instead_of_expect_error(comm.span),
ignore_instead_of_expect_error(comm.content_span()),
|fixer| {
fixer.replace(
comm.span,
comm.content_span(),
raw.cow_replace("@ts-ignore", "@ts-expect-error"),
)
},
);
} else {
ctx.diagnostic(comment(directive, comm.span));
ctx.diagnostic(comment(directive, comm.content_span()));
}
}
}
Expand All @@ -197,7 +197,7 @@ impl Rule for BanTsComment {
ctx.diagnostic(comment_requires_description(
directive,
self.minimum_description_length,
comm.span,
comm.content_span(),
));
}

Expand All @@ -206,7 +206,7 @@ impl Rule for BanTsComment {
ctx.diagnostic(comment_description_not_match_pattern(
directive,
re.as_str(),
comm.span,
comm.content_span(),
));
}
}
Expand Down
Loading

0 comments on commit d1d1874

Please sign in to comment.