From cb2fe378081fcb55896fdc15f642175a8bc97e17 Mon Sep 17 00:00:00 2001 From: fisker Date: Fri, 23 Feb 2024 15:24:36 +0800 Subject: [PATCH 1/3] Add some note to new-rule.md --- docs/new-rule.md | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/docs/new-rule.md b/docs/new-rule.md index 038244ca99..c74a5b1726 100644 --- a/docs/new-rule.md +++ b/docs/new-rule.md @@ -23,3 +23,72 @@ Use the [`astexplorer` site](https://astexplorer.net) with the `espree` parser a - Open a pull request with a title in exactly the format `` Add `rule-name` rule ``, for example, `` Add `no-unused-properties` rule ``. - The pull request description should include the issue it fixes, for example, `Fixes #123`. - Run `npm run run-rules-on-codebase` to run the rules against codebase to ensure code in the repository are following your rule, _you can ignore this step until your PR is reviewed_. + +## Implementation note + +1. Try your best to provide an autofix if possible. +1. Try to provide suggestions if autofix is not possible. +1. Make sure autofix doesn't change the runtime result. +1. Make sure suggestions doesn't cause syntax error. +1. Make sure that edge cases need add parentheses are considered in the fix function. + + ```js + const foo = 1; + foo.toString() + ``` + + When changing `foo` to something else, make sure it works without `()` + + ```js + // Good + (1).toString() + + // Bad, will cause syntax error + 1.toString() + ``` + +1. Make sure that edge cases needs leading semicolons are considered in the fix function. + + ```js + foo + const bar = [1] + bar.forEach(number => { + console.log(number) + }) + ``` + + When changing `bar` to something starts with `[` or `(` + + ```js + // Good + foo + ;[1].forEach(number => { + console.log(number) + }) + + // Bad + foo + [1].forEach(number => { + console.log(number) + }) + ``` + +1. If replacing node can starts or ends with a symbol like `{` make sure to add space before if the replacement starts with a letter. + + The following is valid JavaScript code: + + ```js + for(const{foo}of[]); + ``` + + When replacing `{foo}` with something starts with letter, space around is needed + + ```js + // Good + for(const foo of[]); + + // Bad + for(constfooof[]); + ``` + +1. Try not to remove comments in the fix function. From 1aa368389b8addb2366fbf49fe894a6042c64451 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Fri, 23 Feb 2024 16:32:52 +0700 Subject: [PATCH 2/3] Update new-rule.md --- docs/new-rule.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/new-rule.md b/docs/new-rule.md index c74a5b1726..c6ae463318 100644 --- a/docs/new-rule.md +++ b/docs/new-rule.md @@ -29,15 +29,15 @@ Use the [`astexplorer` site](https://astexplorer.net) with the `espree` parser a 1. Try your best to provide an autofix if possible. 1. Try to provide suggestions if autofix is not possible. 1. Make sure autofix doesn't change the runtime result. -1. Make sure suggestions doesn't cause syntax error. -1. Make sure that edge cases need add parentheses are considered in the fix function. +1. Make sure suggestions don't cause syntax errors. +1. Make sure that edge cases needing parentheses are considered in the fix function. ```js const foo = 1; foo.toString() ``` - When changing `foo` to something else, make sure it works without `()` + When changing `foo` to something else, make sure it works without `()`. ```js // Good @@ -47,7 +47,7 @@ Use the [`astexplorer` site](https://astexplorer.net) with the `espree` parser a 1.toString() ``` -1. Make sure that edge cases needs leading semicolons are considered in the fix function. +1. Make sure that edge cases needing leading semicolons are considered in the fix function. ```js foo @@ -57,7 +57,7 @@ Use the [`astexplorer` site](https://astexplorer.net) with the `espree` parser a }) ``` - When changing `bar` to something starts with `[` or `(` + When changing `bar` to something that starts with `[` or `(`. ```js // Good @@ -73,7 +73,7 @@ Use the [`astexplorer` site](https://astexplorer.net) with the `espree` parser a }) ``` -1. If replacing node can starts or ends with a symbol like `{` make sure to add space before if the replacement starts with a letter. +1. If replacing a node that starts or ends with a symbol like `{`, make sure to add space before if the replacement starts with a letter. The following is valid JavaScript code: @@ -81,7 +81,7 @@ Use the [`astexplorer` site](https://astexplorer.net) with the `espree` parser a for(const{foo}of[]); ``` - When replacing `{foo}` with something starts with letter, space around is needed + When replacing `{foo}` with something that starts with a letter, space around is needed. ```js // Good From 0b6da5eb475e06ec56d7262a8a8fb7802c887950 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Fri, 23 Feb 2024 16:39:07 +0700 Subject: [PATCH 3/3] Update new-rule.md --- docs/new-rule.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/new-rule.md b/docs/new-rule.md index c6ae463318..6465bfdcf6 100644 --- a/docs/new-rule.md +++ b/docs/new-rule.md @@ -27,9 +27,9 @@ Use the [`astexplorer` site](https://astexplorer.net) with the `espree` parser a ## Implementation note 1. Try your best to provide an autofix if possible. -1. Try to provide suggestions if autofix is not possible. -1. Make sure autofix doesn't change the runtime result. -1. Make sure suggestions don't cause syntax errors. +1. Try to provide a suggestion if an autofix is not possible. +1. Make sure the autofix doesn't change the runtime result. +1. Make sure the suggestion doesn't cause a syntax error. 1. Make sure that edge cases needing parentheses are considered in the fix function. ```js