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

Add disambiguating space after unary minus #5949

Open
chfogelman opened this issue Oct 27, 2023 · 3 comments
Open

Add disambiguating space after unary minus #5949

chfogelman opened this issue Oct 27, 2023 · 3 comments

Comments

@chfogelman
Copy link

chfogelman commented Oct 27, 2023

Currently, rustfmt combines unary minus with the expression it's being applied to. This can lead to ambiguity when it's followed by a method call on an integer literal. For example, -1.method() is parsed by rustc as -(1.method()) rather than (-1).method(). To reduce confusion, one might want to see a space between the unary minus and its operand, if the operand is a non-parethesized method invocation on a numeric literal. For example:

-1;
-x;
-(x + y);
-x.y;
-x.method();
-(1.method());
// but
- 1.method();

This issue is motivated by: rust-lang/rust#117155

I'm happy to claim this and implement it, as long as maintainers think it's a good idea.

@rustbot claim

@chfogelman
Copy link
Author

@rustbot label feature-request

@rustbot
Copy link
Collaborator

rustbot commented Oct 27, 2023

Error: The feature relabel is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@ytmimi
Copy link
Contributor

ytmimi commented Oct 27, 2023

@chfogelman thanks for reaching out and offering to help. I've read through the discussion on rust-lang/rust#117155 (thanks for the link).

Right now I'm wondering if there's a clear consensus on what approach, if any, should be taken by rustfmt to resolve the ambiguity. I know you're proposing to add a space, but the issue description suggests rustfmt could stay true to the precedence rules and rewrite -1.method() as -(1.method()).

I think that needs to be cleared up before we move forward with any solution. Just my 2¢, but I think I'd lean towards -(1.method()) since there have been other instances where we needed to wrap expressions in parens to resolve bugs. For example, #5794 and #5842

At the same time I also want to note that this isn't really a bug caused by formatting, so changing how we rewrite the operand of a unary minus operator would need to be version gated or a new configuration option would need to be created where the default behavior is to leave -1.method() as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants