Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(typescript-estree, eslint-plugin): stop adding ParenthesizedExpressions to node maps#226

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

uniqueiniquity
Copy link
Contributor

@uniqueiniquityuniqueiniquity commentedFeb 7, 2019•
edited by armano2
Loading

Builds off#212
The core issue for the crashes is that the TS AST has individual nodes for parenthesized expressions, but the ESTree AST does not.
Assuming a tree of the form(E), whereE is some expression, we would map the TS version ofE to the ESTree version ofE, but the ESTree version ofE would map to the TS version of(E), thereby causing the missingtype property in the repro.
This PR makes it so we don't add TS nodes of typeParenthesizedExpression to the ES->TS map.

I also moved some things around in the particular rule you were investigating; in particular, there's no good reason for us to use the TS node to get the text to check against the provided options, so I switched it to using the ESTree node.

fixes:#187

@uniqueiniquityuniqueiniquity requested review fromJamesHenry andarmano2 and removed request forJamesHenryFebruary 7, 2019 18:15
@uniqueiniquityuniqueiniquity changed the titleHandle parens convertfix(typescript-estree, eslint-plugin): stop adding ParenthesizedExpressions to node mapsFeb 7, 2019
@codecov
Copy link

codecovbot commentedFeb 7, 2019•
edited
Loading

Codecov Report

Merging#226 intomaster willincrease coverage by<.01%.
The diff coverage is100%.

@@            Coverage Diff             @@##           master     #226      +/-   ##==========================================+ Coverage   96.78%   96.79%   +<.01%==========================================  Files          56       56                Lines        2522     2524       +2       Branches      372      373       +1     ==========================================+ Hits         2441     2443       +2  Misses         42       42                Partials       39       39
Impacted FilesCoverage Δ
packages/typescript-estree/src/convert.ts97.9% <100%> (ø)⬆️
...-plugin/lib/rules/no-unnecessary-type-assertion.js100% <100%> (ø)⬆️

Copy link
Collaborator

@armano2armano2 left a comment•
edited
Loading

Choose a reason for hiding this comment

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

this is way better solution than mine, good catch,

besides this small note LGTM

armano2
armano2 previously approved these changesFeb 7, 2019
@codecov
Copy link

codecovbot commentedFeb 7, 2019•
edited
Loading

Codecov Report

Merging#226 intomaster willincrease coverage by<.01%.
The diff coverage is100%.

@@            Coverage Diff             @@##           master     #226      +/-   ##==========================================+ Coverage   96.78%   96.79%   +<.01%==========================================  Files          56       56                Lines        2522     2525       +3       Branches      372      374       +2     ==========================================+ Hits         2441     2444       +3  Misses         42       42                Partials       39       39
Impacted FilesCoverage Δ
...-plugin/lib/rules/no-unnecessary-type-assertion.js100% <100%> (ø)⬆️
packages/typescript-estree/src/convert.ts97.9% <100%> (ø)⬆️

@uniqueiniquity
Copy link
ContributorAuthor

Failing from line ending issues in the fixture

armano2
armano2 previously approved these changesFeb 8, 2019
@uniqueiniquityuniqueiniquity merged commit317405a intotypescript-eslint:masterFeb 8, 2019
@uniqueiniquityuniqueiniquity deleted the handleParensConvert branchFebruary 8, 2019 16:38
kaicataldo pushed a commit to kaicataldo/typescript-eslint that referenced this pull requestAug 27, 2019
…int#226)### Disallow `/// <reference path="" />` commentsbut it's triggered on:- `/// <reference types="foo" />`- `/// <reference lib="es2017.string" />`- `/// <reference no-default-lib="true"/>`types/lib/no-default-lib can't be replaced by es6 imports
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsApr 21, 2020
Luckeu pushed a commit to Luckeu/typescript-eslint that referenced this pull requestJun 27, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@bradzacherbradzacherbradzacher left review comments

@armano2armano2armano2 approved these changes

@JamesHenryJamesHenryAwaiting requested review from JamesHenry

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

False positives forno-unnecessary-type-assertion
3 participants
@uniqueiniquity@armano2@bradzacher
[8]ページ先頭
©2009-2025 Movatter.jp