-
Notifications
You must be signed in to change notification settings - Fork 135
Provider implicit toString #1322
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
Conversation
Generate changelog in
|
| " }", | ||
| "}") | ||
| .doTest(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just add a second test to check that toString on non-providers is not broken.
CRogers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I like this
| name = "GradleProviderToString", | ||
| summary = "Calling toString on a Provider does not render the contained value", | ||
| severity = BugPattern.SeverityLevel.ERROR) | ||
| public final class GradleProviderToString extends AbstractToString { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh this is just too easy :D
| @Override | ||
| protected Optional<Fix> implicitToStringFix(ExpressionTree tree, VisitorState state) { | ||
| // We could automatically call Provider#get, but is that always right? | ||
| return Optional.empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's right enough of the time, we could do it for convenience. We don't need to automatically apply it.
|
This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days. |
…tir/gradle-baseline into ngates/implicit-provider-to-string
|
Think I've addressed comments now |
|
This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days. |
|
Released 3.17.0 |
Before this PR
I was refactoring a Gradle plugin to start using Providers and got hit by this too many times.
After this PR
Implicit use of a Provider as a String is forbidden.
Possible downsides?
No auto-fix