feat(biome_analyzer): support shebang together with // biome-ignore-all file-level suppressions#6712
Conversation
🦋 Changeset detectedLatest commit: 69eeb7e The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CodSpeed Performance ReportMerging #6712 will not alter performanceComparing Summary
|
dyc3
left a comment
There was a problem hiding this comment.
IMO this is the case where practicality beats purity. There is obviously no way to place a comment before shebang.
I agree with this wholeheartedly.
I actually didn't know we parsed shebangs. The tests look good to me, and they pass. Good work!
Well, TIL as well. I didn't know that JS supports shebangs at all - sounds like a weird feature for a language that's so unfit for building CLI aplications... |
Summary
Fixes #6595.
Instead of comparing comment token range with 0, we can check whether it is only preceded by "special" tokens. I added a trait method to represent that, for now it only includes JS_SHEBANG. If necessary, this implementation can be easily extended in future to allow other non-trivial tokens.
While this might cause some counter-intuitive behavior if some lint rules applying to shebang (are there any such rules?) are silenced by a following
// biome-ignore-allcomment, IMO this is the case where practicality beats purity. There is obviously no way to place a comment before shebang. File-level suppressions are defined as comments that affect the whole file, so this should be fine.Test Plan
Added testcases adapted from #6595 example, verified that the shebang example fails on current master and passes here. Added a test with preceding comment, it already passes on master - just added it for consistency.