quick-lint / quick-lint-js

quick-lint-js finds bugs in JavaScript programs
https://quick-lint-js.com
GNU General Public License v3.0
1.52k stars 191 forks source link

10$: warn on 'typeof x === undefined' #1207

Closed strager closed 5 months ago

strager commented 5 months ago

typeof x === undefined is always false. typeof returns a string. The programmer meant typeof x === "undefined" instead.

Master-Helix commented 5 months ago

Hi brother. Can I implement the changes required in this fix? Also, we have to implement the changes in C++ right?

strager commented 5 months ago

@Master-Helix Sure! Here's our contributor guide: https://quick-lint-js.com/contribute/ Let me know if you need more assistance.

Master-Helix commented 5 months ago

We have to make the changes everywhere in the repo for typeof x === undefined right?

strager commented 5 months ago

@Master-Helix What do you mean by 'everywhere in the repo'? For this task you definitely won't need to change (or even look at) most source files in quick-lint-js.

Master-Helix commented 5 months ago

You have tagged it under the C++ domain but it is related to JS files. And I can see the changes required are already there

strager commented 5 months ago

You have tagged it under the C++ domain but it is related to JS files.

quick-lint-js is a JavaScript checker/linter. quick-lint-js is written in C++. For this task you will be modifying C++ code.

And I can see the changes required are already there

I don't know what changes you're referring to. I pasted typeof x === undefined into the quick-lint-js playground and saw no diagnostics/warnings.

Master-Helix commented 5 months ago

You have tagged it under the C++ domain but it is related to JS files.

quick-lint-js is a JavaScript checker/linter. quick-lint-js is written in C++. For this task you will be modifying C++ code.

And I can see the changes required are already there

I don't know what changes you're referring to. I pasted typeof x === undefined into the quick-lint-js playground and saw no diagnostics/warnings.

I understood. Continuing on the fix

Master-Helix commented 5 months ago

You have tagged it under the C++ domain but it is related to JS files.

quick-lint-js is a JavaScript checker/linter. quick-lint-js is written in C++. For this task you will be modifying C++ code.

And I can see the changes required are already there

I don't know what changes you're referring to. I pasted typeof x === undefined into the quick-lint-js playground and saw no diagnostics/warnings.

image

I can see the red warning under typeof.

strager commented 5 months ago

I can see the red warning under typeof.

Hmm, did you make code changes? I don't see what you see:

Screenshot 2024-03-05 at 00 09 23
const x = undefined;

if (typeof x === undefined) {
  console.log("x is undefined");
}
Master-Helix commented 5 months ago

No brother. I am running this directly from your master branch without making any changes. Can you confirm it ?

vegerot commented 5 months ago

I can see the red warning under typeof.

Hmm, did you make code changes? I don't see what you see: Screenshot 2024-03-05 at 00 09 23

const x = undefined;

if (typeof x === undefined) {
  console.log("x is undefined");
}

I can reproduce by typing typeof with the mathematical sans-serif small š¯—‰.

const x = undefined;

if (tyš¯—‰eof x === undefined) {
  console.log("x is undefined");
}

image

Master-Helix commented 5 months ago

I guess the issue is with the font on the playground then ? The syntax should work irrespective of the font used

strager commented 5 months ago

@Master-Helix On the demo, can you run JSON.stringify(document.querySelector('textarea').textContent) in the browser's JS console and share the result? This will help rule out issues like what @vegerot mentioned.

Master-Helix commented 5 months ago

@Master-Helix On the demo, can you run JSON.stringify(document.querySelector('textarea').textContent) in the browser's JS console and share the result? This will help rule out issues like what @vegerot mentioned.

Sure.

image

CoderMuffin commented 5 months ago

I would be interested in attempting to resolve this issue unless either of @Master-Helix or @vegerot wanted to? (I am only asking because I saw the unassignment)

strager commented 5 months ago

@CoderMuffin Sure, you can tackle this task.

CoderMuffin commented 5 months ago

@strager Not sure if you get notified so I just wanted to let you know that I have submitted a pull request with the first draft for these changes :)