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

Feature Request: better expression delimiting for extract variable #4608

Open
Ryan1729 opened this issue Nov 8, 2019 · 2 comments
Open

Feature Request: better expression delimiting for extract variable #4608

Ryan1729 opened this issue Nov 8, 2019 · 2 comments
Labels
improvement subsystem::code insight General label for issues related to code understanding: highlighting, completion, annotation, etc. subsystem::refactoring Issues related to refactorings

Comments

@Ryan1729
Copy link

Ryan1729 commented Nov 8, 2019

Problem description

In Rust if I want to extract a sub-expression into a variable, then I need to wrap it in parens first.
rust_expression_delimiting

In Java though, selecting a sub-expression works as expected.
java_expression_delimiting

Steps to reproduce

In the GIFs I'm pressing Ctrl-alt-v to activate the Extract Variable refactoring after selecting the subexpression.

@artemmukhin artemmukhin added subsystem::code insight General label for issues related to code understanding: highlighting, completion, annotation, etc. improvement labels Nov 14, 2019
@artemmukhin artemmukhin added the subsystem::refactoring Issues related to refactorings label Feb 6, 2020
@Kobzol
Copy link
Member

Kobzol commented Feb 23, 2020

@ortem I looked at it and it seems to be caused by the way the expression is parsed (due to associativity I presume).

1 * <selection>2 * 3</selection> is parsed as BinaryExpr(1*2, 3), so the selection hits "in the middle" and it fails.

1 + <selection>2 * 3</selection> is parsed as BinaryExpr(1, 2 * 3), so the selection finds an expression and the extraction works fine.

Maybe we could duplicate the expression, add fake parentheses, parse it again and then try to extract the expression again?

@Kobzol
Copy link
Member

Kobzol commented Jun 10, 2020

I looked at other languages and it depends on how the PSI is represented. Kotlin also uses BinaryExpr representation, so it has the same problem as the Rust plugin and it cannot extract 1 + <selection>2 * 3</selection>. In Java, a PsiPolyadicExpression is used, which represents an expression with N operands (where N can be more than two), therefore the extraction works, as you have demonstrated.

This seems like a fundamental consequence of the PSI design and it would be very difficult to change this in the Rust plugin. I tried to do some shenanigans like extracting the selection from the expression (1 + <selection>2 * 3</selection> -> 2*3), re-parsing it and creating a new RsExpr out of it and then replacing the original text range, i.e. do not work with it as PSI, but as text. But that seems pretty hacky. @Undin what do you think about this, is it solvable with the current design?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement subsystem::code insight General label for issues related to code understanding: highlighting, completion, annotation, etc. subsystem::refactoring Issues related to refactorings
Projects
None yet
Development

No branches or pull requests

3 participants