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

Fix false positives on non-standard syntax in declaration-block-no-* #3381

Merged
merged 2 commits into from
Jun 15, 2018

Conversation

gucong3000
Copy link
Member

@gucong3000 gucong3000 commented Jun 9, 2018

Which issue, if any, is this issue related to?

#3380
emotion-js/emotion#686 (comment)

Is there anything in the PR that needs further explanation?

No, it's self explanatory.

code:
"margin-left: 10px; margin-right: 10px; margin-top: 20px; margin-bottom: 30px;",
message: messages.expected("margin")
},
Copy link
Member

Choose a reason for hiding this comment

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

Let's add test with html and style tag

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

@gucong3000 Thanks for starting this.

I've added two comments about the tests. The comments apply to all the files e.g. I don't believe the following is valid syntax:

padding-top: 1px; padding-bottom: 1px; a { padding-left: 1px; padding-right: 1px; }

@@ -18,6 +18,10 @@ testRule(rule, {
code: "a { color: pink; { &:hover { color: orange; } } }",
description: "spec nested"
},
{
code: "color: pink; @media { color: orange; }",
Copy link
Member

Choose a reason for hiding this comment

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

As per the developer guide:

Ensure you use standard CSS syntax by default

I don't believe a declaration outside of rule-set is valid. What are we trying to test here?

If it's the content of a style="" attribute then how does this differ from the syntax: html tests below which are more explicit?

@@ -48,6 +52,10 @@ testRule(rule, {
],

reject: [
{
code: "color: pink; color: orange",
Copy link
Member

Choose a reason for hiding this comment

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

If this is for style="" attributes? If so, how does it differ from the syntax: html tests below?

@jeddy3 jeddy3 changed the title Fix: false positives on non-standard syntax in declaration-block-no-* Fix false positives on non-standard syntax in declaration-block-no-* Jun 12, 2018
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

@gucong3000 Thanks for moving the tests to their own testRule. It's much clearer what's going on now and aligns with the checklist:

Ensure you use standard CSS syntax by default, and only swap parsers when testing a specific piece of non-standard syntax.

@evilebottnawi How does this PR now look to you?

@jeddy3 jeddy3 mentioned this pull request Jun 12, 2018
4 tasks
@jeddy3
Copy link
Member

jeddy3 commented Jun 15, 2018

@evilebottnawi If you've time to review this today, then I'll be able to do a release before I go away for the weekend.

@alexander-akait
Copy link
Member

@jeddy3 LGTM

@jeddy3 jeddy3 merged commit 3507bac into master Jun 15, 2018
@jeddy3 jeddy3 deleted the declaration-block branch June 15, 2018 13:12
@jeddy3
Copy link
Member

jeddy3 commented Jun 15, 2018

  • Fixed: declaration-block-no-* false positives for non-standard syntax (#3381).

bors bot referenced this pull request in mozilla/delivery-console Jun 15, 2018
194: Update dependency stylelint to v9.3.0 r=rehandalal a=renovate[bot]

This Pull Request updates dependency [stylelint](https://github.com/stylelint/stylelint) from `v9.2.1` to `v9.3.0`



<details>
<summary>Release Notes</summary>

### [`v9.3.0`](https://github.com/stylelint/stylelint/blob/master/CHANGELOG.md#&#8203;930)
[Compare Source](stylelint/stylelint@9.2.1...9.3.0)
-   Added: support for `<style>` tags and `style=""` attributes in XML and XSLT files ([#&#8203;3386](`https://github.com/stylelint/stylelint/pull/3386`)).
-   Added: `globbyOptions` option ([#&#8203;3339](`https://github.com/stylelint/stylelint/pull/3339`)).
-   Added: `keyframes-name-pattern` rule ([#&#8203;3321](`https://github.com/stylelint/stylelint/pull/3321`)).
-   Added: `media-feature-name-value-whitelist` rule ([#&#8203;3320](`https://github.com/stylelint/stylelint/pull/3320`)).
-   Added: `selector-pseudo-element-colon-notation` autofix ([#&#8203;3345](`https://github.com/stylelint/stylelint/pull/3345`)).
-   Fixed: `.vue` files throwing errors for `<style lang="stylus">` and `<style lang="postcss">` ([#&#8203;3331](`https://github.com/stylelint/stylelint/pull/3331`)).
-   Fixed: `declaration-block-no-*` false positives for non-standard syntax ([#&#8203;3381](`https://github.com/stylelint/stylelint/pull/3381`)).
-   Fixed: `function-whitespace-after` false positives for "/" ([#&#8203;3132](`https://github.com/stylelint/stylelint/pull/3132`)).
-   Fixed: `length-zero-no-unit` incorrect autofix for at-includes ([#&#8203;3347](`https://github.com/stylelint/stylelint/pull/3347`)).
-   Fixed: `max-nesting-depth` false positives for nested properties ([#&#8203;3349](`https://github.com/stylelint/stylelint/pull/3349`)).
-   Fixed: `no-empty-source` false positives on vue external sources `<style src="*">` tag ([#&#8203;3331](`https://github.com/stylelint/stylelint/pull/3331`)).
-   Fixed: `max-line-length` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).
-   Fixed: `no-eol-whitespace` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).
-   Fixed: `no-extra-semicolons` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).
-   Fixed: `no-missing-end-of-source-newline` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).

Co-authored-by: Renovate Bot <bot@renovateapp.com>
schalkneethling referenced this pull request in mdn/interactive-examples Jun 16, 2018
This Pull Request updates dependency [stylelint](https://github.com/stylelint/stylelint) from `v9.2.1` to `v9.3.0`



<details>
<summary>Release Notes</summary>

### [`v9.3.0`](https://github.com/stylelint/stylelint/blob/master/CHANGELOG.md#&#8203;930)
[Compare Source](stylelint/stylelint@9.2.1...9.3.0)
-   Added: support for `<style>` tags and `style=""` attributes in XML and XSLT files ([#&#8203;3386](`https://github.com/stylelint/stylelint/pull/3386`)).
-   Added: `globbyOptions` option ([#&#8203;3339](`https://github.com/stylelint/stylelint/pull/3339`)).
-   Added: `keyframes-name-pattern` rule ([#&#8203;3321](`https://github.com/stylelint/stylelint/pull/3321`)).
-   Added: `media-feature-name-value-whitelist` rule ([#&#8203;3320](`https://github.com/stylelint/stylelint/pull/3320`)).
-   Added: `selector-pseudo-element-colon-notation` autofix ([#&#8203;3345](`https://github.com/stylelint/stylelint/pull/3345`)).
-   Fixed: `.vue` files throwing errors for `<style lang="stylus">` and `<style lang="postcss">` ([#&#8203;3331](`https://github.com/stylelint/stylelint/pull/3331`)).
-   Fixed: `declaration-block-no-*` false positives for non-standard syntax ([#&#8203;3381](`https://github.com/stylelint/stylelint/pull/3381`)).
-   Fixed: `function-whitespace-after` false positives for "/" ([#&#8203;3132](`https://github.com/stylelint/stylelint/pull/3132`)).
-   Fixed: `length-zero-no-unit` incorrect autofix for at-includes ([#&#8203;3347](`https://github.com/stylelint/stylelint/pull/3347`)).
-   Fixed: `max-nesting-depth` false positives for nested properties ([#&#8203;3349](`https://github.com/stylelint/stylelint/pull/3349`)).
-   Fixed: `no-empty-source` false positives on vue external sources `<style src="*">` tag ([#&#8203;3331](`https://github.com/stylelint/stylelint/pull/3331`)).
-   Fixed: `max-line-length` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).
-   Fixed: `no-eol-whitespace` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).
-   Fixed: `no-extra-semicolons` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).
-   Fixed: `no-missing-end-of-source-newline` false positives for non-CSS blocks ([#&#8203;3367](`https://github.com/stylelint/stylelint/pull/3367`)).

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants