feat(eslint-plugin): add rule `no-confusing-non-null-assertion` #1941

zhangciwu · 2020-04-27T12:56:01Z

Fixes #1940
Using a non-null assertions(!) and an equals test(==) together makes code looks very similar with not equals test(!==), look at below code:

a! == b; // a non-null assertions(`!`) and an equals test(`==`)
a !== b; // not equals test(`!==`)
a! === b; // a non-null assertions(`!`) and an triple equals test(`===`)

I have submit an issue on typescript repo Issue: Easy misunderstanding: "! ===", and a PR to fix it.

And I think it's good for many people to avoid this writing by composing a lint rule

Doc at packages/eslint-plugin/docs/rules/confusing-non-null-assertion-like-not-equal.md in this PR

typescript-eslint · 2020-04-27T12:56:08Z

Thanks for the PR, @zhangciwu!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

zhangciwu · 2020-04-27T13:02:22Z

A problem: in the doc, code are altered by git hooks,
(line 36 in confusing-non-null-assertion-like-not-equal.md)

const isEqualsNum = (1 + foo.num!) == 2;
becomes
const isEqualsNum = 1 + foo.num! == 2;

Now I remove code block language describer to avoid this transformation

(Some other rule "fix" it? )

And lack of languege describer cause a markdown lint error……

codecov · 2020-04-27T13:03:58Z

Codecov Report

Merging #1941 into master will decrease coverage by 0.05%.
The diff coverage is 82.60%.

@@            Coverage Diff             @@
##           master    #1941      +/-   ##
==========================================
- Coverage   93.45%   93.40%   -0.06%     
==========================================
  Files         171      172       +1     
  Lines        7813     7844      +31     
  Branches     2227     2239      +12     
==========================================
+ Hits         7302     7327      +25     
- Misses        244      245       +1     
- Partials      267      272       +5

Flag	Coverage Δ
#unittest	`93.40% <82.60%> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
packages/eslint-plugin/src/configs/all.ts	`100.00% <ø> (ø)`
...lugin/src/rules/no-confusing-non-null-assertion.ts	`82.60% <82.60%> (ø)`
...int-plugin/src/rules/require-array-sort-compare.ts	`88.23% <0.00%> (-11.77%)`	⬇️

bradzacher

mostly LGTM.
a few comments.

Also one additional case that should also be handled by this

let x: null | string = null;
x! = 'a';
// looks similar to x != 'a'

// should fix to
x = 'a';

packages/eslint-plugin/src/rules/confusing-non-null-assertion-like-not-equal.ts

packages/eslint-plugin/docs/rules/confusing-non-null-assertion-like-not-equal.md

zhangciwu · 2020-05-19T12:23:38Z

@bradzacher Great idea for x! = 'a';, will work it out; together with rule renaming

…-like-not-equal.md Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

…like-not-equal.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

zhangciwu · 2020-06-05T12:17:42Z

@bradzacher
Done modifing for reviews,

But for CI / Code style and lint (pull_request)
packages/eslint-plugin/docs/rules/no-confusing-non-null-assertion.md L36
const isEqualsNum = (1 + foo.num!) == 2;
This line just can't get through check:format, prettier think the parentheses should be deleted

packages/eslint-plugin/docs/rules/no-confusing-non-null-assertion.md

…ion.md Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

packages/eslint-plugin/tests/rules/no-confusing-non-null-assertion.test.ts

bradzacher

logic lgtm - one change for the tests

…tion.test.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

bradzacher

thanks for your work!

bradzacher added the enhancement: new plugin rule New rule request for eslint-plugin label Apr 27, 2020

bradzacher requested changes May 17, 2020

View reviewed changes

bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label May 17, 2020

zhangciwu and others added 16 commits June 5, 2020 18:03

feat: confusing-non-null-assertion-like-not-equal

b32cfe3

feat: confusing-non-null-assertion-like-not-equal

75f6276

fix: add to config jsons, README

c3fe364

fix: doc

e29cd94

fix: lint

f792eb9

fix: lint

e6ba472

Update packages/eslint-plugin/docs/rules/confusing-non-null-assertion…

f62f7c1

…-like-not-equal.md Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

Update packages/eslint-plugin/src/rules/confusing-non-null-assertion-…

ebc858d

…like-not-equal.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

Update packages/eslint-plugin/src/rules/confusing-non-null-assertion-…

a4ade6e

…like-not-equal.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

Update packages/eslint-plugin/src/rules/confusing-non-null-assertion-…

62f28dd

…like-not-equal.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

fix: rename rule

37ac63e

fix: using BinaryExpression as selector

8f42979

fix: ignore when left hand has parentheses, add tests

8350531

fix: clean code

c4079be

fix: doc

00042bf

fix: insert to all rules

636a4c4

zhangciwu force-pushed the master branch from 43ba878 to 636a4c4 Compare June 5, 2020 10:09

zhangciwu added 2 commits June 5, 2020 19:59

fix: doc

89cba74

fix: prettier

46bc044

zhangciwu requested a review from bradzacher June 5, 2020 12:21

bradzacher reviewed Jun 5, 2020

View reviewed changes

packages/eslint-plugin/docs/rules/no-confusing-non-null-assertion.md Outdated Show resolved Hide resolved

Update packages/eslint-plugin/docs/rules/no-confusing-non-null-assert…

9a70d7c

…ion.md Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

zhangciwu added 3 commits June 6, 2020 15:29

fix: format

163483d

fix: eslint fix

5e4a85e

fix: capitalization

c749e97

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Jun 6, 2020

bradzacher changed the title ~~feat(eslint-plugin): add rule: confusing-non-null-assertion-like-not-equal~~ feat(eslint-plugin): add rule: no-confusing-non-null-assertion Jun 6, 2020

bradzacher reviewed Jun 6, 2020

View reviewed changes

packages/eslint-plugin/tests/rules/no-confusing-non-null-assertion.test.ts Show resolved Hide resolved

bradzacher requested changes Jun 6, 2020

View reviewed changes

bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Jun 6, 2020

zhangciwu and others added 2 commits June 7, 2020 04:36

Update packages/eslint-plugin/tests/rules/no-confusing-non-null-asser…

e32208f

…tion.test.ts Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

fix: use entire file ignore of eslint

88559e5

zhangciwu requested a review from bradzacher June 6, 2020 20:52

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Jun 7, 2020

bradzacher approved these changes Jun 7, 2020

View reviewed changes

bradzacher changed the title ~~feat(eslint-plugin): add rule: no-confusing-non-null-assertion~~ feat(eslint-plugin): add rule no-confusing-non-null-assertion Jun 7, 2020

bradzacher merged commit 9b51c44 into typescript-eslint:master Jun 7, 2020

github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): add rule `no-confusing-non-null-assertion` #1941

feat(eslint-plugin): add rule `no-confusing-non-null-assertion` #1941

zhangciwu commented Apr 27, 2020

typescript-eslint bot commented Apr 27, 2020

zhangciwu commented Apr 27, 2020 •

edited

Loading

codecov bot commented Apr 27, 2020 •

edited

Loading

bradzacher left a comment •

edited

Loading

zhangciwu commented May 19, 2020

zhangciwu commented Jun 5, 2020

bradzacher left a comment

bradzacher left a comment

feat(eslint-plugin): add rule no-confusing-non-null-assertion #1941

feat(eslint-plugin): add rule no-confusing-non-null-assertion #1941

Conversation

zhangciwu commented Apr 27, 2020

typescript-eslint bot commented Apr 27, 2020

zhangciwu commented Apr 27, 2020 • edited Loading

codecov bot commented Apr 27, 2020 • edited Loading

Codecov Report

bradzacher left a comment • edited Loading

Choose a reason for hiding this comment

zhangciwu commented May 19, 2020

zhangciwu commented Jun 5, 2020

bradzacher left a comment

Choose a reason for hiding this comment

bradzacher left a comment

Choose a reason for hiding this comment

feat(eslint-plugin): add rule `no-confusing-non-null-assertion` #1941

feat(eslint-plugin): add rule `no-confusing-non-null-assertion` #1941

zhangciwu commented Apr 27, 2020 •

edited

Loading

codecov bot commented Apr 27, 2020 •

edited

Loading

bradzacher left a comment •

edited

Loading