feature: add LogsafeArgName errorprone rule #1459
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carterkozak
reviewed
Jul 28, 2020
| return buildDescription(tree) | ||
| .setMessage("Arguments with name '" + argName + "' must be marked as unsafe.") | ||
| .addFix(SuggestedFix.builder() | ||
| .replace(tree.getMethodSelect(), "UnsafeArg.of") |
There was a problem hiding this comment.
SuggestedFixes.qualifyType on "com.palantir.logsafe.UnsafeArg" will add the import automagically, or fully qualify the type if there happens to be a foo.bar.UnsafeArg already imported. We can append ".of" to the result.
carterkozak
reviewed
Jul 28, 2020
|
|
||
| public final class LogsafeArgNameTest { | ||
| private CompilationTestHelper compilationHelper; | ||
| private RefactoringValidator refactoringHelper; |
There was a problem hiding this comment.
Slight preference to use factory methods for these, that way we can fluently build the test from the factory.
I recall some of the preconditions tests attempting multiple asserts on a shared helper and getting unexpected results because these guys are stateful!
|
I'd like to merge the associated internal PR first so that excavator will correctly apply the fix on baseline upgrades |
carterkozak
approved these changes
Jul 28, 2020
|
Released 3.36.0 |
This was referenced Jun 22, 2021
This was referenced Sep 15, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this PR
Occasionally developers would incorrectly tag a known unsafe parameter (ex. branch, username) as safe.
After this PR
==COMMIT_MSG==
add LogsafeArgName errorprone rule which allows users to specify a list of argument names that must always be tagged as unsafe.
==COMMIT_MSG==
This isn't a perfect solution since developers may use a different name for an unsafe value, but this should help catch some cases.
The expectation is that we will configure the set of unsafe arg names internally.
Possible downsides?
N/A