Add no-unbalanced-parentheses

alisonailea commented 6 years ago

The Problem

Developers on my team occasionally add extra parenthesis to the end of custom-properties, media-queries, and urls which are not caught by other linting rules and which can cause errors when that CSS is used in other projects.

The Solution

A new rule that checks for extra parenthesis at the end of media queries and custom-properties

Notes

I have a branch in my local clone of Stylelint with this rule already partially written if it is something the community wants I'm happy to finish and submit the PR

Tape test (based off no-extra-semicolons)

"use strict";

const rule = require("..");
const { messages, ruleName } = rule;

testRule(rule, {
  ruleName,
  config: [true],

  accept: [
    {
      code: "a { background: url() }"
    },
    {
      code: "a { background: url('') }"
    },
    {
      code: "a { background: url( ) }"
    },
    {
      code: "a { color: var(--prop) }"
    },
    {
      code: "a { color: (); }"
    },
    {
      code: "a { color: var(--prop);}"
    },
    {
      code: "@media (min-width: var(--prop)) {a { color: var(--prop); }}"
    },
    {
      code: "/* a { color: var(--prop);} */"
    },
    {
      code: "/* a { color: var(--prop)));}; */"
    },
    {
      code: "/* ()comment)) words))(( */"
    },
    {
      code: "a { content: ')'; }"
    },
    {
      code: "a { content: '))'; }"
    },
    {
      code: "a { content: '(\t) )'; }"
    },
    {
      code: ":root { --foo: '('; --bar: '))'; }"
    }
  ],

  reject: [
    {
      code: ")",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: " )",
      message: messages.rejected,
      line: 1,
      column: 2
    },
    {
      code: "  )",
      message: messages.rejected,
      line: 1,
      column: 3
    },
    {
      code: "\n)",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "\n\n)",
      message: messages.rejected,
      line: 3,
      column: 1
    },
    {
      code: "\r\n)",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "\r\n\r\n)",
      message: messages.rejected,
      line: 3,
      column: 1
    },
    {
      code: "\r)",
      message: messages.rejected,
      line: 1,
      column: 2
    },
    {
      code: "\r\r)",
      message: messages.rejected,
      line: 1,
      column: 3
    },
    {
      code: "\t)",
      message: messages.rejected,
      line: 1,
      column: 2
    },
    {
      code: "\t\t)",
      message: messages.rejected,
      line: 1,
      column: 3
    },
    {
      code: "a {)}",
      message: messages.rejected,
      line: 1,
      column: 4
    },
    {
      code: "a { )}",
      message: messages.rejected,
      line: 1,
      column: 5
    },
    {
      code: "a {) }",
      message: messages.rejected,
      line: 1,
      column: 4
    },
    {
      code: "a { ) }",
      message: messages.rejected,
      line: 1,
      column: 5
    },
    {
      code: "a {  )}",
      message: messages.rejected,
      line: 1,
      column: 6
    },
    {
      code: "a {)  }",
      message: messages.rejected,
      line: 1,
      column: 4
    },
    {
      code: "a {  )  }",
      message: messages.rejected,
      line: 1,
      column: 6
    },
    {
      code: "a {   )   }",
      message: messages.rejected,
      line: 1,
      column: 7
    },
    {
      code: "a {\n)}",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "a {)\n}",
      message: messages.rejected,
      line: 1,
      column: 4
    },
    {
      code: "a {\n)\n}",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "a {\r\n)}",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "a {)\r\n}",
      message: messages.rejected,
      line: 1,
      column: 4
    },
    {
      code: "a {\r\n)\r\n}",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "a {\t)}",
      message: messages.rejected,
      line: 1,
      column: 5
    },
    {
      code: "a {)\t}",
      message: messages.rejected,
      line: 1,
      column: 4
    },
    {
      code: "a {\t)\t}",
      message: messages.rejected,
      line: 1,
      column: 5
    },
    {
      code: "a { color: var(--prop); })",
      message: messages.rejected,
      line: 1,
      column: 26
    },
    {
      code: "a { color: var(--prop); } )",
      message: messages.rejected,
      line: 1,
      column: 27
    },
    {
      code: "a { color: var(--prop); }\n)",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "a { color: var(--prop); }\n\n)",
      message: messages.rejected,
      line: 3,
      column: 1
    },
    {
      code: "a { color: var(--prop); }\r\n)",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "a { color: var(--prop); }\r\n\r\n)",
      message: messages.rejected,
      line: 3,
      column: 1
    },
    {
      code: "a { color: var(--prop); }\r)",
      message: messages.rejected,
      line: 1,
      column: 19
    },
    {
      code: "a { color: var(--prop); }\r\r)",
      message: messages.rejected,
      line: 1,
      column: 28
    },
    {
      code: "a { color: var(--prop); }\t)",
      message: messages.rejected,
      line: 1,
      column: 27
    },
    {
      code: "a { color: var(--prop); }\t\t)",
      message: messages.rejected,
      line: 1,
      column: 28
    },
    {
      code: ")a { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ") a { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ")  a { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ")\na { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ")\n\na { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ")\r\na { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ")\r\n\r\na { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ")\ta { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: ")\t\ta { color: var(--prop); }",
      message: messages.rejected,
      line: 1,
      column: 1
    },
    {
      code: "@media ()) { }",
      message: messages.rejected,
      line: 1,
      column: 10
    },
    {
      code: "@media () ) { a { color: var(--prop); } }",
      message: messages.rejected,
      line: 1,
      column: 11
    },
    {
      code: "@media ()  ) { a { color: var(--prop); } }",
      message: messages.rejected,
      line: 1,
      column: 12
    },
    {
      code: "@media (var(--prop))) { a { color: var(--prop); } }",
      message: messages.rejected,
      line: 1,
      column: 21
    },
    {
      code: "@media (var(--prop) )) { a { color: var(--prop); } }",
      message: messages.rejected,
      line: 1,
      column: 22
    },
    {
      code: "@media ( var(--prop) )) { a { color: var(--prop); } }",
      message: messages.rejected,
      line: 1,
      column: 23
    },
    {
      code: "/* comment */)",
      message: messages.rejected,
      line: 1,
      column: 14
    },
    {
      code: "/* comment */ ) /*comment */",
      message: messages.rejected,
      line: 1,
      column: 15
    },
    {
      code: "/* comment */\n)\n/* comment */",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "/* comment */\r\n)\r\n/* comment */",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "/* comment */\t)\t/* comment */",
      message: messages.rejected,
      line: 1,
      column: 15
    },
    {
      code: "a { color: var(--prop)) }",
      message: messages.rejected,
      line: 1,
      column: 23
    },
    {
      code: "a { color: var(--prop) ) }",
      message: messages.rejected,
      line: 1,
      column: 24
    },
    {
      code: "a { color: var(--prop)  ) }",
      message: messages.rejected,
      line: 1,
      column: 25
    },
    {
      code: "a { color: var(--prop)\n) }",
      message: messages.rejected,
      line: 2,
      column: 1
    },

    {
      code: "a { color: var(--prop)\n\n) }",
      message: messages.rejected,
      line: 3,
      column: 1
    },
    {
      code: "a { color: var(--prop)\r\n) }",
      message: messages.rejected,
      line: 2,
      column: 1
    },
    {
      code: "a { color: var(--prop)\r\n\r\n) }",
      message: messages.rejected,
      line: 3,
      column: 1
    },
    {
      code: "a { background: url()) }",
      message: messages.rejected,
      line: 1,
      column: 22
    },
    {
      code: "a { background: url( )) }",
      message: messages.rejected,
      line: 1,
      column: 23
    },
    {
      code: "a { background: url( ) ) }",
      message: messages.rejected,
      line: 1,
      column: 24
    },
    {
      code: "a { background: url('')) }",
      message: messages.rejected,
      line: 1,
      column: 24
    },
    {
      code: "a { background: url(' ') ) }",
      message: messages.rejected,
      line: 1,
      column: 26
    },
    {
      code: "a { @media (min-width: var(--prop))) { color: red; } }",
      message: messages.rejected,
      line: 1,
      column: 36
    },
    {
      code: "a { @media (min-width: var(--prop)) ) { color: red; } }",
      message: messages.rejected,
      line: 1,
      column: 37
    },
    {
      code: "a { @media (min-width: var(--prop) )) { color: red; } }",
      message: messages.rejected,
      line: 1,
      column: 37
    },
    {
      code: "a { @media (min-width: var(--prop ))) { color: red; } }",
      message: messages.rejected,
      line: 1,
      column: 37
    },
    {
      code: "a {\n@media (min-width: var(--prop))) { color: red; } }",
      message: messages.rejected,
      line: 2,
      column: 32
    },
    {
      code: "a {\n@media (\nmin-width: var(--prop))) { color: red; } }",
      message: messages.rejected,
      line: 3,
      column: 24
    },
    {
      code: "a {\n@media (\nmin-width: var(--prop)\n)) { color: red; } }",
      message: messages.rejected,
      line: 4,
      column: 2
    },
  ]
});

testRule(rule, {
  ruleName,
  config: [true],
  syntax: "html",

  accept: [
    {
      code: `<div>
<style>
a {
  color: var(--prop);
}
</style>
</div>`
    },
    {
      code: `<div>
<style>
a {
  background: url();
}
</style>
</div>`
    }
  ],

  reject: [
    {
      code: `<div>
<style>
)
</style>
</div>`,
      message: messages.rejected,
      line: 3,
      column: 1
    },
    {
      code: `<div>
<style>
a {
  color: var(--prop))
}
</style>
</div>`,
      message: messages.rejected,
      line: 4,
      column: 21
    }
  ]
});

testRule(rule, {
  ruleName,
  config: [true],
  syntax: "less",

  accept: [
    {
      code: "a { .mixin(); .mixin2; }"
    },
  ],

  reject: [
    {
      code: "a { .mixin());\ncolor: red; }",
      message: messages.rejected,
      line: 1,
      column: 13
    }
  ]
});

alexander-akait commented 6 years ago

@alisonailea Sound great idea for rule for me

hudochenkov commented 6 years ago

Looks like a good idea! And it meets our criteria for inclusion. Let's wait for @jeddy3, because he usually sees what other miss :)

Even if it won't make to the core, it would make a good plugin.

ntwb commented 6 years ago

Does https://github.com/csstree/stylelint-validator detect this?

Looking at https://csstree.github.io/docs/validator.html many of the examples in the test cases above do throw Parse error: Identifier is expected warnings.

jeddy3 commented 6 years ago

Does https://github.com/csstree/stylelint-validator detect this?

In its current form, it:

will in the value of declarations e.g. functions like calc, url and var etc
won't in the params of at-rules e.g. media and supports etc

CSSTree throws a parse error for the former.

It looks like the PostCSS parser itself will throw a Unclosed bracket or Unknown word parse error for any extra parenthesis outside of the params of an at-rule or the value of a declaration.

In the VISION we have:

Invalid code is best handled by emerging dedicated tools e.g. csstree - a language parser with syntax validation. As a stop-gap, while these tools mature, provide invalid code rules for the simplest of cases.

As the parser itself covers off most instances, I think this will be one of those "simplest cases" and we can add this rule as a stop-gap for the extra parenthesis in the:

params of at-rules for everyone
value of declarations for users of non-standard syntax

@alisonailea Thank you for offering to contribute this rule. You'll find guidance on how to do this in the Developer Guide.

I think you can strip the tests right back, though. Just focus on standard CSS syntax e.g. extra parens in @media, @support, calc, url and var. You can throw in a couple of SCSS syntax tests for @include and @mixin, though.

Name: no-extra-parenthesis
Primary option: true
Secondary options: none
Message: "Unexpected extra parenthesis"
Section: "Possible errors" -> "General / Sheet"

jeddy3 commented 6 years ago

Having thought a little more about this, I think we should limit the scope of this rule to unbalanced parentheses. This is the use case @alisonailea originally describes.

The word extra implies redundant, which would broaden the scope of this rule to include balanced but unnecessary parentheses, e.g. calc((10px) + (10px)), and that isn't viable. This wasn't an issue with semicolons, hence the name no-extra-semicolons.

We should also pluralise the rule name to be consistent with no-extra-semicolons:

Name: no-unbalanced-parentheses
Primary option: true
Secondary options: none
Message: "Unexpected unbalanced parenthesis"
Section: "Possible errors" -> "General / Sheet"

davidmaxwaterman commented 3 years ago

Would this fix the following synthetic case, which just hit me in real code, where I copy/pasted part of a $0.querySelector(....) call I did in chrome devtools as a test, but copied a ')' by mistake and didn't notice?

$ echo ' ) {
  background: red;
}
' | npx stylelint
$

Stylelint didn't flag an error, and the rule didn't apply - took me a while to see that it was a typo.

jeddy3 commented 3 years ago

Would this fix the following synthetic case

PostCSS parses this as a selector of ).

We can expand the scope of the rule to include selectors, so the complete scope becomes:

selectors of rules
params of at-rules
values of declarations

Please consider contributing the rule if you have time.

There are steps on how to add a new rule in the Developer guide.

github-actions[bot] commented 8 months ago

This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.

besalraj30 commented 3 days ago

Hi

I’d like to work on this issue. Could you assign it to me?

stylelint / stylelint