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

fix: function-paren-newline crash on "new new Foo();" #15850

Merged
merged 2 commits into from May 9, 2022

Conversation

coderaiser
Copy link
Contributor

@coderaiser coderaiser commented May 7, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

function-paren-newline crashes when see the code like this:

new new Boolean();

This code has no sense, but this is correct JavaScript:

image

It throws TypeError when trying to execute first new after second one returns instance of an object.

coderaiser@cloudcmd:~/putout/packages/plugin-remove-useless-new$ eslint new-new.js

Oops! Something went wrong! :(

ESLint: 8.15.0

TypeError: Cannot read properties of null (reading 'range')
Occurred while linting /Users/coderaiser/putout/packages/plugin-remove-useless-new/new-new.js:1
Rule: "function-paren-newline"
    at SourceCode.getTokenAfter (/Users/coderaiser/putout/node_modules/eslint/lib/source-code/token-store/index.js:315:18)
    at validateParens (/Users/coderaiser/putout/node_modules/eslint/lib/rules/function-paren-newline.js:109:52)
    at ArrowFunctionExpression,CallExpression,FunctionDeclaration,FunctionExpression,ImportExpression,NewExpression (/Users/coderaiser/putout/node_modules/eslint/lib/rules/function-paren-newline.js:277:21)
    at ruleErrorHandler (/Users/coderaiser/putout/node_modules/eslint/lib/linter/linter.js:1114:28)
    at /Users/coderaiser/putout/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/coderaiser/putout/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/coderaiser/putout/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/Users/coderaiser/putout/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/Users/coderaiser/putout/node_modules/eslint/lib/linter/node-event-generator.js:340:14)
coderaiser@cloudcmd:~/putout/packages/plugin-remove-useless-new$ cat new-new.js
new new Boolean();

Is there anything you'd like reviewers to focus on?

The problem occurs on AwaitExpression switch case when we fall through to CallExpression case

switch (node.type) {
case "NewExpression":
if (!node.arguments.length && !(
astUtils.isOpeningParenToken(sourceCode.getLastToken(node, { skip: 1 })) &&
astUtils.isClosingParenToken(sourceCode.getLastToken(node))
)) {
// If the NewExpression does not have parens (e.g. `new Foo`), return null.
return null;
}
// falls through
case "CallExpression":
return {
leftParen: sourceCode.getTokenAfter(node.callee, astUtils.isOpeningParenToken),
rightParen: sourceCode.getLastToken(node)
};

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 7, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: coderaiser / name: coderaiser (d42e865)

@eslint-github-bot eslint-github-bot bot added triage An ESLint team member will look at this issue soon bug ESLint is working incorrectly labels May 7, 2022
@netlify
Copy link

netlify bot commented May 7, 2022

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit dc0854e
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/62791b83398d050009bd0c18

@aladdin-add
Copy link
Member

@coderaiser can you sign the cla?

@coderaiser
Copy link
Contributor Author

@coderaiser can you sign the cla?

Sure, I did it a couple times, but for some reason I always get redirected to eslint/eslint-visitor-keys#32 after sign up.

@snitin315
Copy link
Contributor

@coderaiser please make sure you are committing using the same email id which you used to sign the CLA. Please check your local git settings.

@coderaiser
Copy link
Contributor Author

@coderaiser please make sure you are committing using the same email id which you used to sign the CLA. Please check your local git settings.

I tried all my emails actually, and did it about 10 times 😅. Looks like something isn’t working properly, I can’t understand why I get redirected to already merged PR in other repo 🤷.

@aladdin-add
Copy link
Member

aladdin-add commented May 9, 2022

I can’t understand why I get redirected to already merged PR in other repo 🤷.

I guess it's another OpenJS project, and is using the easyCLA too. 😂

@coderaiser
Copy link
Contributor Author

I guess it's another OpenJS project, and is using the easyCLA too. 😂

But how this separate project related to Missing ID on Commit button I'm clicking opening the link https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/19114526/11061773/15850/#/?version=2.

Does this easyCLA remember projects that was signed before? Maybe this link is misconfigured?

@aladdin-add
Copy link
Member

https://github.com/openjs-foundation/EasyCLA#openjs-foundation-cla
Once a contributor is authorized for one OpenJS Foundation project, the signature is recognized for any other OpenJS Foundation repos that use EasyCLA.

@aladdin-add
Copy link
Member

looks happy now. 👍

@mdjermanovic mdjermanovic added rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels May 9, 2022
lib/rules/function-paren-newline.js Outdated Show resolved Hide resolved
@mdjermanovic mdjermanovic changed the title fix: function-paren-newline: crash on "new new Foo();" fix: function-paren-newline crash on "new new Foo();" May 9, 2022
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

I'll leave this open until we conclude the current release (#15805).

@mdjermanovic mdjermanovic merged commit 3f09aab into eslint:main May 9, 2022
@mdjermanovic
Copy link
Member

Thanks for contributing!

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request May 24, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.15.0` -> `8.16.0`](https://renovatebot.com/diffs/npm/eslint/8.15.0/8.16.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.16.0`](https://github.com/eslint/eslint/releases/tag/v8.16.0)

[Compare Source](eslint/eslint@v8.15.0...v8.16.0)

#### Features

-   [`cab0c22`](eslint/eslint@cab0c22) feat: add Unicode flag suggestion in no-misleading-character-class ([#&#8203;15867](eslint/eslint#15867)) (Milos Djermanovic)
-   [`38ae956`](eslint/eslint@38ae956) feat: check Unicode code point escapes in no-control-regex ([#&#8203;15862](eslint/eslint#15862)) (Milos Djermanovic)
-   [`ee69cd3`](eslint/eslint@ee69cd3) feat: Update global variables ([#&#8203;15871](eslint/eslint#15871)) (Sébastien Règne)

#### Bug Fixes

-   [`3f09aab`](eslint/eslint@3f09aab) fix: function-paren-newline crash on "new new Foo();" ([#&#8203;15850](eslint/eslint#15850)) (coderaiser)

#### Documentation

-   [`050d5f4`](eslint/eslint@050d5f4) docs: Static further reading links ([#&#8203;15890](eslint/eslint#15890)) (Nicholas C. Zakas)
-   [`36287c0`](eslint/eslint@36287c0) docs: fix absolute paths in related rules shortcode to work from /docs ([#&#8203;15892](eslint/eslint#15892)) (Milos Djermanovic)
-   [`90b6990`](eslint/eslint@90b6990) docs: fix absolute links in rule macro to work from /docs ([#&#8203;15891](eslint/eslint#15891)) (Milos Djermanovic)
-   [`f437249`](eslint/eslint@f437249) docs: Adjust docs site path prefix ([#&#8203;15889](eslint/eslint#15889)) (Nicholas C. Zakas)
-   [`6e16025`](eslint/eslint@6e16025) docs: update 'Related Rules' and 'Further Reading' in remaining rules ([#&#8203;15884](eslint/eslint#15884)) (Milos Djermanovic)
-   [`1d39f69`](eslint/eslint@1d39f69) docs: remove confusing examples for no-mixed-operators ([#&#8203;15875](eslint/eslint#15875)) (Milos Djermanovic)
-   [`3071d76`](eslint/eslint@3071d76) docs: Fix some grammar issues ([#&#8203;15837](eslint/eslint#15837)) (byodian)

#### Chores

-   [`1768d0d`](eslint/eslint@1768d0d) chore: upgrade [@&#8203;eslint/eslintrc](https://github.com/eslint/eslintrc)[@&#8203;1](https://github.com/1).3.0 ([#&#8203;15903](eslint/eslint#15903)) (Milos Djermanovic)
-   [`c686e4c`](eslint/eslint@c686e4c) chore: Add deploy workflow for docs site ([#&#8203;15894](eslint/eslint#15894)) (Nicholas C. Zakas)
-   [`c7894cd`](eslint/eslint@c7894cd) chore: enable some rules from eslint-plugin-unicorn internally ([#&#8203;15878](eslint/eslint#15878)) (Bryan Mishkin)
-   [`ea65cb5`](eslint/eslint@ea65cb5) chore: upgrade eslint-plugin-eslint-plugin@^4.2.0 ([#&#8203;15882](eslint/eslint#15882)) (唯然)
-   [`cc29c69`](eslint/eslint@cc29c69) chore: Upgrade official GitHub actions to latest versions ([#&#8203;15880](eslint/eslint#15880)) (Darius Dzien)
-   [`5891c75`](eslint/eslint@5891c75) chore: Refactor rule docs format ([#&#8203;15869](eslint/eslint#15869)) (Nicholas C. Zakas)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1366
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
srijan-deepsource pushed a commit to DeepSourceCorp/eslint that referenced this pull request May 30, 2022
* fix: function-paren-newline: crash on "new new Foo();"

* fix: function-paren-newline: false negative case support
srijan-deepsource added a commit to DeepSourceCorp/eslint that referenced this pull request May 30, 2022
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Nov 6, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants