Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
strager opened this issue Mar 3, 2024 · 17 comments
Closed

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

strager opened this issue Mar 3, 2024 · 17 comments
Assignees
Labels
for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html good first issue Good for newcomers and C++ beginners

Comments

@strager
Copy link
Collaborator

strager commented Mar 3, 2024

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

@strager strager added good first issue Good for newcomers and C++ beginners for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html labels Mar 3, 2024
@Master-Helix
Copy link

Master-Helix commented Mar 3, 2024

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

@strager
Copy link
Collaborator Author

strager commented Mar 4, 2024

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

@Master-Helix
Copy link

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

@strager
Copy link
Collaborator Author

strager commented Mar 5, 2024

@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
Copy link

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
Copy link
Collaborator Author

strager commented Mar 5, 2024

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
Copy link

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
Copy link

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
Copy link
Collaborator Author

strager commented Mar 5, 2024

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
Copy link

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

@vegerot
Copy link
Contributor

vegerot commented Mar 5, 2024

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
Copy link

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

@strager
Copy link
Collaborator Author

strager commented Mar 5, 2024

@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
Copy link

@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
Copy link
Contributor

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
Copy link
Collaborator Author

strager commented Mar 11, 2024

@CoderMuffin Sure, you can tackle this task.

@CoderMuffin
Copy link
Contributor

@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 :)

@strager strager closed this as completed Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for hire Get paid for working on this task: https://quick-lint-js.com/hiring.html good first issue Good for newcomers and C++ beginners
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants