Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): implement @typescript-eslint/triple-slash-reference #1903

Merged
merged 11 commits into from
Jan 11, 2024

Conversation

kaykdm
Copy link
Contributor

@kaykdm kaykdm commented Jan 5, 2024

@github-actions github-actions bot added the A-linter Area - Linter label Jan 5, 2024
Copy link

codspeed-hq bot commented Jan 5, 2024

CodSpeed Performance Report

Merging #1903 will not alter performance

Comparing kaykdm:triple-slash-reference (dc71ea4) with main (6e0bd52)

Summary

✅ 14 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Jan 6, 2024

I think we can do this, given TSImportEqualsDeclaration and ImportDeclaration only appear at top level:

for each statement i in program.body
  if the statement is an import
    get all comments from i and i-1
      check for triple slash

For getting comments, use the range API from BTreeMap: comments.range(span.start..span.end)

@kaykdm
Copy link
Contributor Author

kaykdm commented Jan 6, 2024

I think we can do this, given TSImportEqualsDeclaration and ImportDeclaration only appear at top level:

for each statement i in program.body
  if the statement is an import
    get all comments from i and i-1
      check for triple slash

For getting comments, use the range API from BTreeMap: comments.range(span.start..span.end)

@Boshen
Thank you!
I will fix that to use program.body instead of ctx.nodes().
However, even we use program.body to find import statement and iterate comment with range, we still need to check other comments because triple-slash directives are not always with import statement.

Also, it made me realize that I don't need to iterate all comments since Triple-slash directives are only valid at the top of their containing file.
I should find the first program node to get start value and use it to iterate comments with range.
It seems like typescript-eslint uses program node's range to get comments.
https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/rules/triple-slash-reference.ts#L100

@kaykdm
Copy link
Contributor Author

kaykdm commented Jan 7, 2024

@Boshen
I have done the following

  • use program.body instead of ctx.nodes() to find TSImportEqualsDeclaration and ImportDeclaration
  • find the start value of the first statement and use it for comments iteration range
  • iterate comments first and then program.body to find import statement if necessary

I am not sure why performance regression is happening 🤔
I think not that much processing is going on in both checker.ts and pdf.mjs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants