Do not change case of property name if inside a variable declaration in LESS #14034

mvorisek · 2022-12-18T14:33:04Z

Description

Simple example:

@var: {
  camelCase: 0;
};

div {
  margin: @var[camelCase];
};

when preserveCase is changed to preservecase, LESS compilation fails.

This PR fixes such problem and handles also variable declaration with nested ruleset correctly.

Checklist

I’ve added tests to confirm my change works.
(If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
(If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

Compared to Stylelint, Prettier is very simple tool and I am quite supprised Stylelint is deprecating the whitespace rules in favor of Prettier. For LESS it seems ok, for JS, we should never enable Prettier as formatting is part of a code and improves readability. As Prettier is an opinionated formatter instead of an linter, we need to satisfy it everywhere without exceptions. When I was crafting this PR, I found several Prettier issues and proposed changes to Prettier: Fix semicolon duplicated at the end of LESS file prettier/prettier#14007 Fix no space after unary minus when followed by opening parenthesis in LESS prettier/prettier#14008 Do not change case of property name if inside a variable declaration in LESS prettier/prettier#14034 In this PR, these changes are contained and Prettier is patched before it is run. Once the changes are merged in prettier and stable release is made, they can be removed. This PR fixes minor whitespaces unfixed/undetected in GH-2610. And also asserts: PHP port of the LESS fails to compile colors.less #1832 feat(lint): add stylelint overrides and variation files #2593 (comment) Prettier has no built in support to dump the diff - prettier/prettier#6885 - so we dump it in the CI using git.

fisker · 2022-12-29T02:45:01Z

@mvorisek The tests are failing.

Tests failure

mvorisek · 2022-12-29T08:50:30Z

@mvorisek The tests are failing.

@fisker any idea how to fix it and why does it fail on Node 18 only?

fisker · 2022-12-29T08:55:30Z

Use FULL_TEST to debug https://github.com/prettier/prettier/blob/main/CONTRIBUTING.md#deeper-testing

mvorisek · 2022-12-29T10:22:55Z

(my/added) test fixed, thanks for FULL_TEST hint

fisker · 2022-12-29T11:59:34Z

What cases the AST changes? I can't see what you changed.

mvorisek · 2022-12-29T14:28:36Z

Test to compile the LESS code from the PR description with camelCase property name changed to camelcase. LESS compilation will fail, case in a declaration must be preserved as it is case sensitive when referenced.

fisker · 2022-12-29T15:06:11Z

I know what are we fixing, I mean how did you fix the FULL_TEST, your code changes the AST at first, that's why the test failing. I want know how did you fix that.

mvorisek · 2022-12-29T15:08:55Z

sure, removed this not needed testcase I initially proposed:

fisker · 2022-12-29T15:12:40Z

I'm not familiar with LESS. Is it valid LESS code? Should we fix it instead of removing it.

mvorisek · 2022-12-29T15:20:39Z

here is minimalistic test case:

@foo : {
  bar: 0;
};

div {
  margin: @foo[bar];
};

in https://lesscss.org/less-preview/ I can compile it /w and /wo space to the same result

in https://prettier.io/playground/ when space is removed, the AST is different

not sure if really a bug in Prettier, maybe a bug in LESS parser? It is not related with this PR nor my needs, so feel free to pursue it, but I have no capacity to do so.

fisker · 2022-12-29T15:25:29Z

Can you add this test back, I can look into it later.

fisker · 2022-12-29T15:26:59Z

The difference is .value of css-atrule, right?

fisker · 2022-12-29T15:32:47Z

Thanks.

mvorisek · 2022-12-29T15:39:58Z

The difference is .value of css-atrule, right?

I have added two failing test cases:

first is definitely in Prettier and should be easy to fix

2nd is the problem we discussed here, ie. different AST when at rule has a whitespace between colon:

-@var2 : {
+@var2: {
   notVar: 5;
 };

fisker · 2022-12-29T15:42:17Z

I'll take a look later, maybe tomorrow.

fisker · 2022-12-30T02:17:54Z

tests/format/less/case/__snapshots__/jsfmt.spec.js.snap

+  shouldnotchangecase: @var[ @notVarNested][notVar];
+  shouldnotchangecase: @var[ @notVarNested ][notVar];
+  shouldnotchangecase: @var[ @notVarNested][ notVar];
+  shouldnotchangecase: @var[ @notVarNested][notVar ];


We are fine with these spaces, right?

I would expect no trailing space, especially when space is not present in the original. Can it be fixed easily?

I didn't notice the space is added.

src/language-css/printer-postcss.js

sosukesuzuki · 2022-12-30T08:18:24Z

@mvorisek Are you okay with changes by @\fisker ? If you don't have concerns we can merge this PR.

mvorisek · 2022-12-30T09:38:33Z

#14034 (comment) would be nice to be fixed as well, otherwise I have no objections.

fisker · 2022-12-30T09:45:48Z

#14034 (comment)

Let's merge this first, we can try to fix it in other PR.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [prettier](https://prettier.io) ([source](https://github.com/prettier/prettier)) | devDependencies | patch | [`2.8.1` -> `2.8.2`](https://renovatebot.com/diffs/npm/prettier/2.8.1/2.8.2) | --- ### Release Notes <details> <summary>prettier/prettier</summary> ### [`v2.8.2`](https://github.com/prettier/prettier/blob/HEAD/CHANGELOG.md#282) [Compare Source](prettier/prettier@2.8.1...2.8.2) [diff](prettier/prettier@2.8.1...2.8.2) ##### Don't lowercase link references ([#13155](prettier/prettier#13155) by [@DerekNonGeneric](https://github.com/DerekNonGeneric) & [@fisker](https://github.com/fisker))  ```markdown  We now don't strictly follow the release notes format suggested by [Keep a Changelog]. [Keep a Changelog]: https://example.com/  We now don't strictly follow the release notes format suggested by [Keep a Changelog]. [keep a changelog]: https://example.com/   <Same as input> ``` ##### Preserve self-closing tags ([#13691](prettier/prettier#13691) by [@dcyriller](https://github.com/dcyriller))  ```hbs {{! Input }} <div /> <div></div> <custom-component /> <custom-component></custom-component> <i /> <i></i> <Component /> <Component></Component> {{! Prettier 2.8.1 }} <div></div> <div></div> <custom-component></custom-component> <custom-component></custom-component> <i></i> <i></i> <Component /> <Component /> {{! Prettier 2.8.2 }} <div /> <div></div> <custom-component /> <custom-component></custom-component> <i /> <i></i> <Component /> <Component /> ``` ##### Allow custom "else if"-like blocks with block params ([#13930](prettier/prettier#13930) by [@jamescdavis](https://github.com/jamescdavis)) [#13507](prettier/prettier#13507) added support for custom block keywords used with `else`, but failed to allow block params. This updates printer-glimmer to allow block params with custom "else if"-like blocks.  ```hbs {{! Input }} {{#when isAtWork as |work|}} Ship that {{work}}! {{else when isReading as |book|}} You can finish {{book}} eventually... {{else}} Go to bed! {{/when}} {{! Prettier 2.8.1 }} {{#when isAtWork as |work|}} Ship that {{work}}! {{else when isReading}} You can finish {{book}} eventually... {{else}} Go to bed! {{/when}} {{! Prettier 2.8.2 }} {{#when isAtWork as |work|}} Ship that {{work}}! {{else when isReading as |book|}} You can finish {{book}} eventually... {{else}} Go to bed! {{/when}} ``` ##### Preserve empty lines between nested SCSS maps ([#13931](prettier/prettier#13931) by [@jneander](https://github.com/jneander))  ```scss /* Input */ $map: ( 'one': ( 'key': 'value', ), 'two': ( 'key': 'value', ), ) /* Prettier 2.8.1 */ $map: ( 'one': ( 'key': 'value', ), 'two': ( 'key': 'value', ), ) /* Prettier 2.8.2 */ $map: ( 'one': ( 'key': 'value', ), 'two': ( 'key': 'value', ), ) ``` ##### Fix missing parentheses when an expression statement starts with `let[` ([#14000](prettier/prettier#14000), [#14044](prettier/prettier#14044) by [@fisker](https://github.com/fisker), [@thorn0](https://github.com/thorn0))  ```jsx // Input (let[0] = 2); // Prettier 2.8.1 let[0] = 2; // Prettier 2.8.1 (second format) SyntaxError: Unexpected token (1:5) > 1 | let[0] = 2; | ^ 2 | // Prettier 2.8.2 (let)[0] = 2; ``` ##### Fix semicolon duplicated at the end of LESS file ([#14007](prettier/prettier#14007) by [@mvorisek](https://github.com/mvorisek))  ```less // Input @variable: { field: something; }; // Prettier 2.8.1 @variable: { field: something; }; ; // Prettier 2.8.2 @variable: { field: something; }; ``` ##### Fix no space after unary minus when followed by opening parenthesis in LESS ([#14008](prettier/prettier#14008) by [@mvorisek](https://github.com/mvorisek))  ```less // Input .unary_minus_single { margin: -(@a); } .unary_minus_multi { margin: 0 -(@a); } .binary_minus { margin: 0 - (@a); } // Prettier 2.8.1 .unary_minus_single { margin: - (@a); } .unary_minus_multi { margin: 0 - (@a); } .binary_minus { margin: 0 - (@a); } // Prettier 2.8.2 .unary_minus_single { margin: -(@a); } .unary_minus_multi { margin: 0 -(@a); } .binary_minus { margin: 0 - (@a); } ``` ##### Do not change case of property name if inside a variable declaration in LESS ([#14034](prettier/prettier#14034) by [@mvorisek](https://github.com/mvorisek))  ```less // Input @var: { preserveCase: 0; }; // Prettier 2.8.1 @var: { preservecase: 0; }; // Prettier 2.8.2 @var: { preserveCase: 0; }; ``` ##### Fix formatting for auto-accessors with comments ([#14038](prettier/prettier#14038) by [@fisker](https://github.com/fisker))  ```jsx // Input class A { @dec() // comment accessor b; } // Prettier 2.8.1 class A { @dec() accessor // comment b; } // Prettier 2.8.1 (second format) class A { @dec() accessor; // comment b; } // Prettier 2.8.2 class A { @dec() // comment accessor b; } ``` ##### Add parentheses for TSTypeQuery to improve readability ([#14042](prettier/prettier#14042) by [@onishi-kohei](https://github.com/onishi-kohei))  ```tsx // Input a as (typeof node.children)[number] a as (typeof node.children)[] a as ((typeof node.children)[number])[] // Prettier 2.8.1 a as typeof node.children[number]; a as typeof node.children[]; a as typeof node.children[number][]; // Prettier 2.8.2 a as (typeof node.children)[number]; a as (typeof node.children)[]; a as (typeof node.children)[number][]; ``` ##### Fix displacing of comments in default switch case ([#14047](prettier/prettier#14047) by [@thorn0](https://github.com/thorn0)) It was a regression in Prettier 2.6.0.  ```jsx // Input switch (state) { default: result = state; // no change break; } // Prettier 2.8.1 switch (state) { default: // no change result = state; break; } // Prettier 2.8.2 switch (state) { default: result = state; // no change break; } ``` ##### Support type annotations on auto accessors via `babel-ts` ([#14049](prettier/prettier#14049) by [@sosukesuzuki](https://github.com/sosukesuzuki)) [The bug that `@babel/parser` cannot parse auto accessors with type annotations](babel/babel#15205) has been fixed. So we now support it via `babel-ts` parser.  ```tsx class Foo { accessor prop: number; } ``` ##### Fix formatting of empty type parameters ([#14073](prettier/prettier#14073) by [@fisker](https://github.com/fisker))  ```jsx // Input const foo: bar</* comment */> = () => baz; // Prettier 2.8.1 Error: Comment "comment" was not printed. Please report this error! // Prettier 2.8.2 const foo: bar</* comment */> = () => baz; ``` ##### Add parentheses to head of `ExpressionStatement` instead of the whole statement ([#14077](prettier/prettier#14077) by [@fisker](https://github.com/fisker))  ```jsx // Input ({}).toString.call(foo) === "[object Array]" ? foo.forEach(iterateArray) : iterateObject(foo); // Prettier 2.8.1 ({}.toString.call(foo) === "[object Array]" ? foo.forEach(iterateArray) : iterateObject(foo)); // Prettier 2.8.2 ({}).toString.call(foo.forEach) === "[object Array]" ? foo.forEach(iterateArray) : iterateObject(foo); ``` ##### Fix comments after directive ([#14081](prettier/prettier#14081) by [@fisker](https://github.com/fisker))  ```jsx // Input "use strict" /* comment */; // Prettier 2.8.1 (with other js parsers except `babel`) Error: Comment "comment" was not printed. Please report this error! // Prettier 2.8.2 <Same as input> ``` ##### Fix formatting for comments inside JSX attribute ([#14082](prettier/prettier#14082) with by [@fisker](https://github.com/fisker))  ```jsx // Input function MyFunctionComponent() { <button label=/*old*/"new">button</button> } // Prettier 2.8.1 Error: Comment "old" was not printed. Please report this error! // Prettier 2.8.2 function MyFunctionComponent() { <button label=/*old*/ "new">button</button>; } ``` ##### Quote numeric keys for json-stringify parser ([#14083](prettier/prettier#14083) by [@fisker](https://github.com/fisker))  ```jsx // Input {0: 'value'} // Prettier 2.8.1 { 0: "value" } // Prettier 2.8.2 { "0": "value" } ``` ##### Fix removing commas from function arguments in maps ([#14089](prettier/prettier#14089) by [@sosukesuzuki](https://github.com/sosukesuzuki))  ```scss /* Input */ $foo: map-fn( ( "#{prop}": inner-fn($first, $second), ) ); /* Prettier 2.8.1 */ $foo: map-fn(("#{prop}": inner-fn($first $second))); /* Prettier 2.8.2 */ $foo: map-fn( ( "#{prop}": inner-fn($first, $second), ) ); ``` ##### Do not insert space in LESS property access ([#14103](prettier/prettier#14103) by [@fisker](https://github.com/fisker))  ```less // Input a { color: @colors[@white]; } // Prettier 2.8.1 a { color: @colors[ @white]; } // Prettier 2.8.2 <Same as input> ``` </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1708 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

mvorisek changed the title ~~Do not change case of property name if inside a variable declaration~~ Do not change case of property name if inside a variable declaration in LESS Dec 18, 2022

mvorisek force-pushed the fix_no_less_lower_inside_variable_declaration branch from 821cbeb to d31e8d3 Compare December 18, 2022 14:36

mvorisek marked this pull request as ready for review December 18, 2022 14:41

mvorisek mentioned this pull request Dec 18, 2022

feat(lint): add prettier formatter to assert LESS files whitespaces fomantic/Fomantic-UI#2611

Merged

fisker previously approved these changes Dec 19, 2022

View reviewed changes

Do not change case of property name if inside a variable declaration

8d22001

mvorisek force-pushed the fix_no_less_lower_inside_variable_declaration branch from 64a1c5c to 8d22001 Compare December 29, 2022 09:58

break tests to an expected format

a0c3742

break tests to an expected format II

978334b

fisker added 3 commits December 30, 2022 10:07

Skip parsing empty text

c8e4f49

More tests

7224af6

Add more tests

c6d23dd

fisker reviewed Dec 30, 2022

View reviewed changes

fisker added 3 commits December 30, 2022 10:18

Clean

c5e0f77

shouldNotChangeCase -> preserveCase

7379800

One more test

f5c87e5

fisker reviewed Dec 30, 2022

View reviewed changes

src/language-css/printer-postcss.js Outdated Show resolved Hide resolved

fisker added 3 commits December 30, 2022 10:26

Use .variable to check

fd414fb

Add TODO

d8b72f0

Fix test

0027b2f

fisker approved these changes Dec 30, 2022

View reviewed changes

sosukesuzuki approved these changes Dec 30, 2022

View reviewed changes

fisker merged commit 41cee06 into prettier:main Jan 3, 2023
28 checks passed

fisker mentioned this pull request Jan 3, 2023

Stop inserting space in LESS property access #14103

Merged

4 tasks

mvorisek deleted the fix_no_less_lower_inside_variable_declaration branch January 3, 2023 10:13

Do not change case of property name if inside a variable declaration in LESS #14034

Do not change case of property name if inside a variable declaration in LESS #14034

mvorisek commented Dec 18, 2022 •

edited

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022 •

edited

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022 •

edited

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022 •

edited

fisker commented Dec 29, 2022

fisker commented Dec 29, 2022

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022 •

edited

fisker Dec 30, 2022

mvorisek Dec 30, 2022

fisker Dec 30, 2022

sosukesuzuki commented Dec 30, 2022

mvorisek commented Dec 30, 2022

fisker commented Dec 30, 2022

Do not change case of property name if inside a variable declaration in LESS #14034

Do not change case of property name if inside a variable declaration in LESS #14034

Conversation

mvorisek commented Dec 18, 2022 • edited

Description

Checklist

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022 • edited

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022 • edited

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022 • edited

fisker commented Dec 29, 2022

fisker commented Dec 29, 2022

fisker commented Dec 29, 2022

mvorisek commented Dec 29, 2022

fisker commented Dec 29, 2022 • edited

fisker Dec 30, 2022

Choose a reason for hiding this comment

mvorisek Dec 30, 2022

Choose a reason for hiding this comment

fisker Dec 30, 2022

Choose a reason for hiding this comment

sosukesuzuki commented Dec 30, 2022

mvorisek commented Dec 30, 2022

fisker commented Dec 30, 2022

mvorisek commented Dec 18, 2022 •

edited

fisker commented Dec 29, 2022 •

edited

fisker commented Dec 29, 2022 •

edited

mvorisek commented Dec 29, 2022 •

edited

fisker commented Dec 29, 2022 •

edited