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

cmd/gofmt: Remove redundant parenthesis in foo = (1 << 42) #26706

Open
RalphCorderoy opened this issue Jul 31, 2018 · 5 comments
Open

cmd/gofmt: Remove redundant parenthesis in foo = (1 << 42) #26706

RalphCorderoy opened this issue Jul 31, 2018 · 5 comments

Comments

@RalphCorderoy
Copy link

@RalphCorderoy RalphCorderoy commented Jul 31, 2018

go version go1.10.3 linux/amd64

Reviewing CLs showed lists of const bit-masks that were foo = 1 << 42, etc., all had redundant parenthesis around the (1 << 42). The author said they were copying similar ones in existing source. gofmt doesn't remove them. It does remove the outer pair of double ones, i.e. ((1 << 314)) in https://play.golang.org/p/V6jz7iKHJwt

I think simple cases like foo = (1 << 42) can always have them removed. That's the style followed by most of the stdlib, but not all. Having pointed it out on the CL, Martin Möhrmann is fixing those other cases that influenced the author, e.g. https://go-review.googlesource.com/126775, but it would be nice if gofmt avoided the possibility.

@martisch
Copy link
Contributor

@martisch martisch commented Jul 31, 2018

I send CLs to make the style of internal/cpu consistent. I do not plan to rewrite any other occurrences in the Go std library.

@agnivade
Copy link
Contributor

@agnivade agnivade commented Jul 31, 2018

@mvdan
Copy link
Member

@mvdan mvdan commented Jul 31, 2018

This can already be done with gofmt, as documented in the examples https://golang.org/cmd/gofmt/#hdr-Examples:

gofmt -r '(a) -> a' -w *.go

Perhaps the -s flag could take care of this, but I don't think that the regular gofmt is designed to do cleanups like these. All the changes that add or remove syntax should be done by -r or -s, following the current design.

@RalphCorderoy
Copy link
Author

@RalphCorderoy RalphCorderoy commented Jul 31, 2018

Is there a standard list of -r that are applied to golang.org source for consistency?
The suggested -r '(a) -> a' rewrites more than I'm suggesting. It: anywhere the () are redundant. Me: the redundant () when they're outermost on an ExpressionList.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 31, 2018

I agree that this could be done if the -s option is given, but not otherwise.

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

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.