Skip to content

fix: prevent false positives for !important inside comments #218

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Aug 4, 2025
Merged

fix: prevent false positives for !important inside comments #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf36665
fix: prevent false positives for !important inside comments
thecalamiity Jul 30, 2025
1ef48a0
add tests for !important comment edge cases
thecalamiity Aug 1, 2025
0bfdc0c
Merge branch 'main' into no-important-ignore-comments
thecalamiity Aug 1, 2025
ad35008
add missing suggestions to tests
thecalamiity Aug 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/rules/no-important.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import { findOffsets } from "../util.js";
// Helpers
//-----------------------------------------------------------------------------

const importantPattern = /!(\s|\/\*.*?\*\/)*important/iu;
const importantPattern = /!\s*important/iu;
Copy link

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern for matching CSS comments should include a comment explaining its purpose and behavior, especially the non-greedy matching with *? which is crucial for handling multiple comments correctly.

Suggested change
const importantPattern = /!\s*important/iu;
const importantPattern = /!\s*important/iu;
/**
* Regex pattern to match CSS comments.
* Uses non-greedy matching (*?) to ensure that multiple comments are matched correctly,
* preventing the pattern from spanning across multiple comment blocks.
* Matches anything between /* and *\/, including newlines.
*/

Copilot uses AI. Check for mistakes.
const commentPattern = /\/\*[\s\S]*?\*\//gu;
const trailingWhitespacePattern = /\s*$/u;

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -55,8 +56,12 @@ export default {
Declaration(node) {
if (node.important) {
const declarationText = context.sourceCode.getText(node);
Copy link

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nested replace operation to preserve newlines while replacing other characters with spaces is complex and may be unclear to future maintainers. Consider adding a comment explaining why newlines need to be preserved (likely for accurate line/column positioning).

Suggested change
const declarationText = context.sourceCode.getText(node);
const declarationText = context.sourceCode.getText(node);
// Replace all non-newline characters in comments with spaces, preserving newlines.
// This ensures that line/column offsets remain accurate for error reporting,
// since removing newlines would shift positions in the source text.

Copilot uses AI. Check for mistakes.
const textWithoutComments = declarationText.replace(
commentPattern,
match => match.replace(/[^\n]/gu, " "),
);
const importantMatch =
importantPattern.exec(declarationText);
importantPattern.exec(textWithoutComments);

const {
lineOffset: startLineOffset,
Expand Down
229 changes: 229 additions & 0 deletions tests/rules/no-important.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ ruleTester.run("no-important", rule, {
"@keyframes important { from { margin: 1px; } }",
"@-webkit-keyframes important { from { margin: 1px; } }",
"@-WEBKIT-KEYFRAMES important { from { margin: 1px; } }",
"a { color: red /* !important */; }",
"a { color: /* !important */ red; }",
"a { color: red; /* !important */ background: blue; }",
],
invalid: [
{
Expand Down Expand Up @@ -535,5 +538,231 @@ ruleTester.run("no-important", rule, {
},
],
},
{
code: "a { color: red /* !important */ !important; }",
errors: [
{
messageId: "unexpectedImportant",
line: 1,
column: 33,
endLine: 1,
endColumn: 43,
suggestions: [
{
messageId: "removeImportant",
output: "a { color: red /* !important */; }",
},
],
},
],
},
{
code: dedent`
a {
color: red /* !important */
!important;
}
`,
errors: [
{
messageId: "unexpectedImportant",
line: 3,
column: 3,
endLine: 3,
endColumn: 13,
suggestions: [
{
messageId: "removeImportant",
output: dedent`
a {
color: red /* !important */;
}
`,
},
],
},
],
},
{
code: "a { color: red !/* !important */important; }",
errors: [
{
messageId: "unexpectedImportant",
line: 1,
column: 16,
endLine: 1,
endColumn: 42,
suggestions: [
{
messageId: "removeImportant",
output: "a { color: red; }",
},
],
},
],
},
{
code: "a { color: red ! /* !important */ important; }",
errors: [
{
messageId: "unexpectedImportant",
line: 1,
column: 16,
endLine: 1,
endColumn: 44,
suggestions: [
{
messageId: "removeImportant",
output: "a { color: red; }",
},
],
},
],
},
{
code: dedent`
a {
color: red
!/* !important */important;
}
`,
errors: [
{
messageId: "unexpectedImportant",
line: 3,
column: 3,
endLine: 3,
endColumn: 29,
suggestions: [
{
messageId: "removeImportant",
output: dedent`
a {
color: red;
}
`,
},
],
},
],
},
{
code: dedent`
a {
color: red
! /* !important */ important;
}
`,
errors: [
{
messageId: "unexpectedImportant",
line: 3,
column: 3,
endLine: 3,
endColumn: 31,
suggestions: [
{
messageId: "removeImportant",
output: dedent`
a {
color: red;
}
`,
},
],
},
],
},
{
code: "a { color: red /* !important */ /* !important */ !important; }",
errors: [
{
messageId: "unexpectedImportant",
line: 1,
column: 50,
endLine: 1,
endColumn: 60,
suggestions: [
{
messageId: "removeImportant",
output: "a { color: red /* !important */ /* !important */; }",
},
],
},
],
},
{
code: dedent`
a {
color: red /* !important */ /* !important
*/ !important;
}
`,
errors: [
{
messageId: "unexpectedImportant",
line: 3,
column: 5,
endLine: 3,
endColumn: 15,
suggestions: [
{
messageId: "removeImportant",
output: dedent`
a {
color: red /* !important */ /* !important
*/;
}
`,
},
],
},
],
},
{
code: "a { color: red ! /* !important */ /* another comment */ /* a third comment */important; }",
errors: [
{
messageId: "unexpectedImportant",
line: 1,
column: 16,
endLine: 1,
endColumn: 87,
suggestions: [
{
messageId: "removeImportant",
output: "a { color: red; }",
},
],
},
],
},
{
code: dedent`
a {
color: red ! /* !important */ /* another
comment */ /* a third comment */important;
}
`,
errors: [
{
messageId: "unexpectedImportant",
line: 2,
column: 13,
endLine: 3,
endColumn: 43,
suggestions: [
{
messageId: "removeImportant",
output: dedent`
a {
color: red;
}
`,
},
],
},
],
},
],
});
Loading