feat: fixable match-component-file-name rule #1874

mahaker · 2022-04-29T07:02:43Z

This plugin has always helped me.
Thank you very much.

There are two fixes.

Add 'Literal#raw' property
fixable match-component-file-name rule

vue-eslint-parser's ESLintLiteralBase Node has 'raw' property. ref: https://github.com/vuejs/vue-eslint-parser/blob/160f4efc4eaf363662b464a4a26a4c9e514deb5d/src/ast/nodes.ts#L395 It was necessary to identify quotes in the match-component-file-name's fixer.

mahaker · 2022-04-29T07:06:35Z

lib/rules/match-component-file-name.js

@@ -102,19 +102,25 @@ module.exports = {
     */
    function verifyName(node) {
      let name
+      let quote = ''


Defining let quote, resulted in a typescript error.

Variable 'quote' implicitly has type 'any' in some locations where its type cannot be determined.

You could instead set a type without initializing the variable:

/** @type {string} */ let quote // equivalent to the following Typescript syntax: let quote: string

But maybe it would make sense to only compute the quote in the fixer function:

fix(fixer) { const quote = node.type === 'TemplateLiteral' ? '`' : node.raw[0] return fixer.replaceText(node, `${quote}${filename}${quote}`) }

Then you don't have to initialize it and the meaning of the variable is immediately clear from the context.

mahaker · 2022-04-29T07:12:12Z

typings/eslint-plugin-vue/util-types/ast/es-ast.ts

@@ -347,6 +347,7 @@ export interface PrivateIdentifier extends HasParentNode {
 export interface Literal extends HasParentNode {
  type: 'Literal'
  value: string | boolean | null | number | RegExp | BigInt
+  raw: string


console.log(node), the following object can be displayed.
The fixer use the raw property to determine the single/doublequotes.

<ref *2> Node { type: 'Literal', start: 38, end: 51, loc: SourceLocation { start: { line: 3, column: 10 }, end: { line: 3, column: 23 } }, range: [ 38, 51 ], value: 'MyComponent', raw: "'MyComponent'", parent: <ref *1> Node { type: 'Property', start: 32, end: 51, loc: SourceLocation { start: [Object], end: [Object] }, range: [ 32, 51 ], method: false, shorthand: false, computed: false, key: Node { type: 'Identifier', start: 32, end: 36, loc: [SourceLocation], range: [Array], name: 'name', parent: [Circular *1] }, value: [Circular *2], kind: 'init', parent: Node { type: 'ObjectExpression', start: 26, end: 110, loc: [SourceLocation], range: [Array], properties: [Array], parent: [Node] } } }

I found Literal's raw property is defined here.

FloEdelmann · 2022-04-29T10:40:30Z

lib/rules/match-component-file-name.js

+          fix(fixer) {
+            return fixer.replaceText(node, `${quote}${filename}${quote}`)
+          }


I think this should not be an auto-fix, since it's sometimes the case that the filename is wrong and the name property is correct.

Instead, please change this to be a suggestion instead. Then cases where the filename is correct and the name property is wrong can be easily corrected in the editor, without causing wrong fixes when running from the commandline.

The suggestion description should be "Rename component to match file name.", as described in the issue #1816.

Thank you very much. I understand the reason to using suggest instead of fix.

FloEdelmann · 2022-04-29T10:41:56Z

lib/rules/match-component-file-name.js

@@ -102,19 +102,25 @@ module.exports = {
     */
    function verifyName(node) {
      let name
+      let quote = ''


You could instead set a type without initializing the variable:

/** @type {string} */ let quote // equivalent to the following Typescript syntax: let quote: string

FloEdelmann · 2022-04-29T10:46:23Z

lib/rules/match-component-file-name.js

@@ -102,19 +102,25 @@ module.exports = {
     */
    function verifyName(node) {
      let name
+      let quote = ''


But maybe it would make sense to only compute the quote in the fixer function:

fix(fixer) { const quote = node.type === 'TemplateLiteral' ? '`' : node.raw[0] return fixer.replaceText(node, `${quote}${filename}${quote}`) }

Then you don't have to initialize it and the meaning of the variable is immediately clear from the context.

FloEdelmann · 2022-04-29T10:50:02Z

Thanks for your first contribution! I have a few comments, see above. After you're done, please run npm run update to update the docs (which adds a hint that the rule has suggestions).

mahaker · 2022-05-01T04:37:00Z

Very thanks for the review!!

I will confirm comments, and fix them.
Also run npm run update to update the docs.

mahaker · 2022-05-01T06:24:06Z

I force pushed commit. Could you please review it?
If the review is ok, i will run npm run update to update the docs.

I have checked the operation of suggest with my VSCode and ESLint extension.
It seems to be working correctly.

1 2: `Quick Fix` clicked

3: `Rename component to...` clicked

ota-meshi

Thank you for this PR!
Almost LGTM! I have only one request to change the test script.

ota-meshi · 2022-05-02T06:23:45Z

tests/lib/rules/match-component-file-name.js

        }
-      ]
+      ],
+      output: null


Remove output as it is no longer needed.

Very thanks for the review!!

I removed output, and run update the document.

improve vuejs#1816 For the following reasons why name option changed. If change file name, alto need to change import statement. Also, this rule not auto-fixed, because the file name may be incorrect.

FloEdelmann

Looks good to me, thanks!

ota-meshi

LGTM! thank you!

mahaker commented Apr 29, 2022

View reviewed changes

FloEdelmann requested changes Apr 29, 2022

View reviewed changes

mahaker force-pushed the fixable-match-component-file-name-rule branch from 2bd2cb4 to 0a9a584 Compare May 1, 2022 06:14

ota-meshi requested changes May 2, 2022

View reviewed changes

feat: add suggestion match-component-file-name rule

4ff89bb

improve vuejs#1816 For the following reasons why name option changed. If change file name, alto need to change import statement. Also, this rule not auto-fixed, because the file name may be incorrect.

mahaker force-pushed the fixable-match-component-file-name-rule branch from 0a9a584 to 4ff89bb Compare May 4, 2022 02:27

FloEdelmann approved these changes May 4, 2022

View reviewed changes

FloEdelmann requested a review from ota-meshi May 7, 2022 15:13

ota-meshi approved these changes May 9, 2022

View reviewed changes

ota-meshi merged commit 4daf4c8 into vuejs:master May 9, 2022

mahaker deleted the fixable-match-component-file-name-rule branch May 9, 2022 08:50

FloEdelmann mentioned this pull request May 9, 2022

Summary of eslint-plugin-vue v9 #1850

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: fixable match-component-file-name rule #1874

feat: fixable match-component-file-name rule #1874

mahaker commented Apr 29, 2022 •

edited

Loading

mahaker Apr 29, 2022

FloEdelmann Apr 29, 2022

FloEdelmann Apr 29, 2022

mahaker Apr 29, 2022

mahaker May 1, 2022

FloEdelmann Apr 29, 2022 •

edited

Loading

mahaker May 1, 2022

FloEdelmann Apr 29, 2022

FloEdelmann Apr 29, 2022

FloEdelmann commented Apr 29, 2022

mahaker commented May 1, 2022

mahaker commented May 1, 2022

ota-meshi left a comment

ota-meshi May 2, 2022

mahaker May 4, 2022

FloEdelmann left a comment

ota-meshi left a comment

feat: fixable match-component-file-name rule #1874

feat: fixable match-component-file-name rule #1874

Conversation

mahaker commented Apr 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FloEdelmann Apr 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FloEdelmann commented Apr 29, 2022

mahaker commented May 1, 2022

mahaker commented May 1, 2022

1

2: Quick Fix clicked

3: Rename component to... clicked

ota-meshi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FloEdelmann left a comment

Choose a reason for hiding this comment

ota-meshi left a comment

Choose a reason for hiding this comment

mahaker commented Apr 29, 2022 •

edited

Loading

FloEdelmann Apr 29, 2022 •

edited

Loading

2: `Quick Fix` clicked

3: `Rename component to...` clicked