Skip to content

Commit

Permalink
Migrate preconditions to the new package
Browse files Browse the repository at this point in the history
See palantir/safe-logging#515

This change does two things:
* The existing errorprone checks which reference preconditions
  have been updated to suggest the new package name
* A new `SafeLoggingPreconditionsMigration` check has been added
  to automatically migrate code to the new package when a
  sufficiently new `safe-logging` version is available.
  • Loading branch information
carterkozak committed Jan 26, 2021
1 parent 28aa63d commit d462236
Show file tree
Hide file tree
Showing 11 changed files with 229 additions and 57 deletions.
2 changes: 1 addition & 1 deletion .baseline/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
</module>
<module name="AvoidStarImport"/> <!-- Java Style Guide: No wildcard imports -->
<module name="AvoidStaticImport"> <!-- Java Style Guide: No static imports -->
<property name="excludes" value="com.google.common.base.Preconditions.*, com.palantir.logsafe.Preconditions.*, java.util.Collections.*, java.util.stream.Collectors.*, org.apache.commons.lang3.Validate.*, org.assertj.core.api.Assertions.*, org.mockito.Mockito.*"/>
<property name="excludes" value="com.google.common.base.Preconditions.*, com.palantir.logsafe.Preconditions.*, com.palantir.logsafe.preconditions.Preconditions.*, java.util.Collections.*, java.util.stream.Collectors.*, org.apache.commons.lang3.Validate.*, org.assertj.core.api.Assertions.*, org.mockito.Mockito.*"/>
</module>
<module name="ClassTypeParameterName"> <!-- Java Style Guide: Type variable names -->
<property name="format" value="(^[A-Z][0-9]?)$|([A-Z][a-zA-Z0-9]*[T]$)"/>
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
- `PreferSafeLoggingPreconditions`: Users should use the safe-logging versions of Precondition checks for standardization when there is equivalent functionality
```diff
-com.google.common.base.Preconditions.checkNotNull(variable, "message");
+com.palantir.logsafe.Preconditions.checkNotNull(variable, "message"); // equivalent functionality is available in the safe-logging variant
+com.palantir.logsafe.preconditions.Preconditions.checkNotNull(variable, "message"); // equivalent functionality is available in the safe-logging variant
```
- `ShutdownHook`: Applications should not use `Runtime#addShutdownHook`.
- `GradleCacheableTaskAction`: Gradle plugins should not call `Task.doFirst` or `Task.doLast` with a lambda, as that is not cacheable. See [gradle/gradle#5510](https://github.com/gradle/gradle/issues/5510) for more details.
Expand Down Expand Up @@ -202,6 +202,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
- `IncubatingMethod`: Prevents calling Conjure incubating APIs unless you explicitly opt-out of the check on a per-use or per-project basis.
- `CompileTimeConstantViolatesLiskovSubstitution`: Requires consistent application of the `@CompileTimeConstant` annotation to resolve inconsistent validation based on the reference type on which the met is invoked.
- `ClassInitializationDeadlock`: Detect type structures which can cause deadlocks initializing classes.
- `SafeLoggingPreconditionsMigration`: Migrate safe-logging preconditions references to the new non-deprecated package.

### Programmatic Application

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public final class LogSafePreconditionsMessageFormat extends PreconditionsMessag
private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> LOGSAFE_PRECONDITIONS_METHOD = MethodMatchers.staticMethod()
.onClassAny("com.palantir.logsafe.Preconditions")
.onClassAny("com.palantir.logsafe.preconditions.Preconditions")
.withNameMatching(Pattern.compile("checkArgument|checkState|checkNotNull"));

public LogSafePreconditionsMessageFormat() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION,
severity = BugPattern.SeverityLevel.WARNING,
summary = "Precondition and similar checks with a constant message and no parameters should use equivalent"
+ " checks from com.palantir.logsafe.Preconditions for standardization as functionality is the"
+ " checks from com.palantir.logsafe.preconditions.Preconditions for standardization as functionality is the"
+ " same.")
public final class PreferSafeLoggingPreconditions extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {

Expand Down Expand Up @@ -92,13 +92,15 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
}

SuggestedFix.Builder fix = SuggestedFix.builder();
String logSafeQualifiedClassName = SuggestedFixes.qualifyType(state, fix, "com.palantir.logsafe.Preconditions");
String logSafeQualifiedClassName =
SuggestedFixes.qualifyType(state, fix, "com.palantir.logsafe.preconditions.Preconditions");
String logSafeMethodName = getLogSafeMethodName(ASTHelpers.getSymbol(tree));
String replacement = String.format("%s.%s", logSafeQualifiedClassName, logSafeMethodName);

return buildDescription(tree)
.setMessage("The call can be replaced with an equivalent one from com.palantir.logsafe.Preconditions "
+ "for standardization as the functionality is the same.")
.setMessage(
"The call can be replaced with an equivalent one from com.palantir.logsafe.preconditions.Preconditions "
+ "for standardization as the functionality is the same.")
.addFix(fix.replace(tree.getMethodSelect(), replacement).build())
.build();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.baseline.errorprone;

import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.Tree;

@AutoService(BugChecker.class)
@BugPattern(
name = "SafeLoggingPreconditionsMigration",
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = BugPattern.LinkType.CUSTOM,
severity = SeverityLevel.SUGGESTION,
summary = "Prefer the non-deprecated safe-logging preconditions path. "
+ "See https://github.com/palantir/safe-logging/pull/515 for context. Using gradle baseline, "
+ "failures can be fixed automatically using "
+ "`./gradlew classes testClasses -PerrorProneApply=SafeLoggingPreconditionsMigration`")
public final class SafeLoggingPreconditionsMigration extends BugChecker implements BugChecker.MemberSelectTreeMatcher {

private static final Matcher<Tree> LEGACY_PRECONDITIONS_MATCHER =
Matchers.isSameType("com.palantir.logsafe.Preconditions");

@Override
public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) {
if (LEGACY_PRECONDITIONS_MATCHER.matches(tree, state)
// Only attempt to warn or refactor when the preconditions dependency version is sufficiently new,
// older versions which haven't deprecated "com.palantir.logsafe.Preconditions" won't have the
// replacement.
&& ASTHelpers.hasAnnotation(ASTHelpers.getSymbol(tree), Deprecated.class, state)) {
return buildDescription(tree)
.addFix(SuggestedFix.replace(tree, "com.palantir.logsafe.preconditions.Preconditions"))
.build();
}
return Description.NO_MATCH;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public final void failLogSafe(String diagnostic, String precondition) {
compilationHelper()
.addSourceLines(
"Test.java",
"import com.palantir.logsafe.Preconditions;",
"import com.palantir.logsafe.preconditions.Preconditions;",
"import com.palantir.logsafe.UnsafeArg;",
"class Test {",
" void f(String param) {",
Expand Down Expand Up @@ -149,7 +149,7 @@ public final void validLogSafe() throws Exception {
compilationHelper()
.addSourceLines(
"Test.java",
"import com.palantir.logsafe.Preconditions;",
"import com.palantir.logsafe.preconditions.Preconditions;",
"import com.palantir.logsafe.UnsafeArg;",
"import java.util.Iterator;",
"class Test {",
Expand Down
Loading

0 comments on commit d462236

Please sign in to comment.