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/ban-ts-comment #741

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

Devin-Yeung
Copy link
Contributor

related to #503, marked as nursery(though all tests pass) since we use rust regex crate to parse the regex, may have compatibility problems with ECMA regex.

@github-actions github-actions bot added the A-linter Area - Linter label Aug 14, 2023
@Boshen
Copy link
Member

Boshen commented Aug 15, 2023

The regex seems pretty simple to implement as a tiny lexer.

I have this from my internal code, which may or may not be correct:

impl Trivia {
    /// For TypeScript comment directive `@ts-ignore` / `@ts-expect-error` / `ts-nocheck` / `ts-check`,
    /// Find and return the matching span witin the comment
    #[must_use]
    pub fn find_ts_comment_directive(&self, source_text: &str) -> Option<Span> {
        let prefix = "@ts-";
        let range = self.range();
        let raw = &source_text[range];
        // For multi-line comments, get the last line
        let mut lines = raw.lines();
        let line = lines.next_back()?;
        let index = line.find(prefix)?;
        if !line[..index].chars().all(|c| {
            c.is_whitespace()
                || match self.kind() {
                    // allow `//////@ts-` for SingleLineComment
                    TriviaKind::SingleLineComment => c == '/',
                    // allow `/*****@ts-` for MultiLineComment
                    TriviaKind::MultiLineComment => c == '*' || c == '/',
                }
        }) {
            return None;
        }
        let multi_len = lines.map(|line| line.len() + 1).sum::<usize>();
        let start = index + prefix.len();
        for directive in ["expect-error", "ignore", "nocheck", "check"] {
            if line.get(start..start + directive.len()) == Some(directive) {
                let start = multi_len + index;
                let end = start + prefix.len() + directive.len();
                return Some(start..end);
            }
        }
        None
    }
}

Would you like to try and implement the regex as a tiny lexer? I can also merge this and leave the refactoring to the next person.

@Devin-Yeung
Copy link
Contributor Author

Looks pretty cool! I think I want to get this PR merged and maybe give it a try in another PR.

@Boshen Boshen merged commit 607fa6a into oxc-project:main Aug 15, 2023
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.

2 participants