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 if expression range #94

Merged
merged 4 commits into from
Jan 24, 2021

Conversation

MartinSStewart
Copy link
Collaborator

This fixes #92.

As @jfmengels mentioned, this is indeed related to an earlier let...in range miscalculation. I think there might be more bugs like this (though I didn't find anymore right now) as it's caused by Node.parser which uses the current parser position as the end position of a range rather than using the end position of the child expression. For many cases, the current parser position and the end position of the expression are the same. But in let, case, and if expressions, this isn't true.

@MartinSStewart
Copy link
Collaborator Author

@jfmengels It just occurred to me that maybe I should have made this PR for your backport-fixes-to-v7 branch instead of master. Let me know what you think.

Copy link
Collaborator

@jfmengels jfmengels left a comment

Choose a reason for hiding this comment

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

LGTM 👍
I've tried it out on elm-review rules that had bugs around this, and they now pass.

I'll push these commits directly on the branch for #93

@jfmengels
Copy link
Collaborator

It just occurred to me that maybe I should have made this PR for your backport-fixes-to-v7 branch instead of master. Let me know what you think.

Keep on doing them on master. Cherry-picking them on v7 will rarely be difficult I think.

@jfmengels jfmengels merged commit 6314578 into stil4m:master Jan 24, 2021
jfmengels pushed a commit that referenced this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Range miscalculation when using If expressions
2 participants