Repository navigation
Don't rewrite at-rules mentioned inside CSS comments - #1625
Open
SahinurDEV wants to merge 4 commits into
Open
SahinurDEV wants to merge 4 commits into
SahinurDEV wants to merge 4 commits into
Conversation
The CSS language server rewrites Tailwind at-rules with regexes before
handing documents to vscode-css-languageservice. Those regexes also matched
at-rule names written inside comments, and since `[^{]+` spans newlines a
mention like `@variant` in a comment would swallow everything up to the
next `{`, including the `*/`. The rest of the file was then parsed as a
comment and bogus diagnostics showed up far away from the cause.
Skip matches that start or end inside a comment, and make the style rule
and `@custom-variant …;` replacements keep the line breaks they consume so
positions after a multi-line prelude stay correct.
Fixes tailwindlabs#1615
[Medium impact] The PR appears safe to merge; the previous finding is fixed and no new issues were found. Reviews (3) · Last reviewed commit: "Don't treat #url( and @url( as unquoted ..." · Reviewed by Greptile |
Also keeps columns intact for a multiline @custom-variant whose semicolon starts a line, and resumes matching at the end of a comment instead of rescanning it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1615
The CSS language server rewrites Tailwind's at-rules with a series of regexes (
rewriteCssinpackages/tailwindcss-language-server/src/language/rewriting.ts) before it hands the document tovscode-css-languageservice. Those regexes don't know about comments, so an at-rule name mentioned in a comment gets rewritten too. For the{-terminated ones the prelude is[^{]+, which runs across newlines up to the next{. In the repro from the issue,@varianton line 2 eats everything up to the{of@themeon line 4,*/included. The comment never gets closed, the parser treats the file as a comment until the next*/, and you get{ expectedon a perfectly valid.a {.There's a second problem that makes this much harder to trace:
replaceWithStyleRule(used for@utility,@theme,@custom-variant … {and@variant) and the@custom-variant …;replacement pad the prelude with spaces, newlines included. So every rewrite that spans lines moves everything below it up, and diagnostics land on the wrong line.replaceWithAtRulealready keeps the line breaks; the other two didn't.Changes
replaceOutsideComments(), which works like a globalString#replacebut leaves alone any match that starts or ends inside a/* … */comment. Comment ranges come from a small scanner that skips quoted strings, socontent: "/*"doesn't open a comment. When it skips a match it picks up again at the next character, not at the end of the skipped match. Otherwise a match that starts in a comment could hide a real at-rule right after it (/* @variant */\n@variant foo {still gets rewritten). A comment that sits entirely inside a real prelude (@utility bar /* note */ {) gets blanked out the same way it did before.rewriteCssnow go through it.replaceWithStyleRuleand the@custom-variant …;replacement now keep the line breaks they consume, the same wayreplaceWithAtRuledoes, so positions after a multi-line prelude stay correct.Tests
src/language/rewriting.test.ts: at-rules (and--foo-*) mentioned in comments are left untouched, real at-rules after/around such comments (and after a string containing/*) are still rewritten, and multi-line preludes keep their line count.tests/css/css-server.test.ts: the exact repro from the issue now has no diagnostics, and an error after multi-line@utility/@custom-variantpreludes is reported on the right line (it was off by two before).All five new tests fail on
mainand pass with this change.vitest run src/language tests/csspasses (35 tests). I also ran the full language-server suite: everything passes exceptsrc/project-locator.test.tsand the#-path test intests/env/v4.test.js, which fail the same way onmainon my machine (fixture/environment issue, nothing to do with this change).tsc --noEmitreports no new errors.