Skip to content

Conversation

asgerf
Copy link
Contributor

@asgerf asgerf commented Jun 15, 2020

Improves the alert message in cases where the offending variable looks like a type annotation:

For example:

function distance({x: number, y: number}) {
    return Math.sqrt(x*x + y*y);
}

should actually be:

function distance({x, y}: {x: number, y: number}) {
    return Math.sqrt(x*x + y*y);
}

One could argue this type of alert belongs in IneffectiveParameterType.ql but this isn't a priority.

@asgerf asgerf added the JS label Jun 15, 2020
@asgerf asgerf requested review from mchammer01 and a team as code owners June 15, 2020 16:40
Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

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

👍

One could argue this type of alert belongs in IneffectiveParameterType.ql but this isn't a priority.

I think it fits well in NonLinearPattern.ql.

Can you do a small performance test on smoke-tests before merge?

@asgerf asgerf added the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Jun 16, 2020
@asgerf
Copy link
Contributor Author

asgerf commented Jun 17, 2020

Evaluation looks fine.

@asgerf asgerf removed the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Jun 17, 2020
@erik-krogh erik-krogh merged commit cd111fe into github:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants