Skip to content

Commit

Permalink
Include check for logsafe Preconditions (#256)
Browse files Browse the repository at this point in the history
  • Loading branch information
cbrockington authored and Robert Fink committed Mar 29, 2018
1 parent 17b906f commit 628aa7c
Show file tree
Hide file tree
Showing 12 changed files with 613 additions and 126 deletions.
5 changes: 0 additions & 5 deletions .baseline/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,6 @@
<property name="format" value="\? extends Object\W"/>
<property name="message" value="Use ? rather than ? extends Object."/>
</module>
<module name="RegexpSinglelineJava">
<property name="format" value="\bPreconditions\.check.*\{\}"/>
<property name="message" value="Use printf-style formatting in Preconditions, not '{}' style formatting."/>
<property name="ignoreComments" value="true"/>
</module>
<module name="RegexpSinglelineJava">
<property name="format" value="(?i)log(ger)?\.(debug|info|warn|error)\(.*%[sd]"/>
<property name="message" value="SLF4J loggers support '{}' style formatting."/>
Expand Down
1 change: 1 addition & 0 deletions baseline-error-prone/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ dependencies {

testCompile 'com.fasterxml.jackson.core:jackson-annotations'
testCompile 'com.google.errorprone:error_prone_test_helpers'
testCompile 'com.palantir.safe-logging:preconditions'
testCompile 'com.palantir.safe-logging:safe-logging'
testCompile 'org.slf4j:slf4j-api'
testCompile 'org.apache.commons:commons-lang3'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* (c) Copyright 2018 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.Category;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.matchers.method.MethodMatchers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import java.util.regex.Pattern;

@AutoService(BugChecker.class)
@BugPattern(
name = "GuavaPreconditionsMessageFormat",
category = Category.GUAVA,
severity = SeverityLevel.ERROR,
summary = "Guava Preconditions.checkX() methods must use print-f style formatting.")
public final class GuavaPreconditionsMessageFormat extends PreconditionsMessageFormat {

private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> GUAVA_PRECONDITIONS_METHODS =
Matchers.anyOf(
MethodMatchers.staticMethod()
.onClassAny("com.google.common.base.Preconditions")
.withNameMatching(Pattern.compile("checkArgument|checkState|checkNotNull")));

public GuavaPreconditionsMessageFormat() {
super(GUAVA_PRECONDITIONS_METHODS);
}

@Override
protected Description matchMessageFormat(MethodInvocationTree tree, String message, VisitorState state) {
if (!message.contains("{}")) {
return Description.NO_MATCH;
}

return buildDescription(tree).setMessage(
"Use printf-style formatting in Guava Preconditions, not '{}' style formatting.").build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* (c) Copyright 2018 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.Category;
import com.google.errorprone.BugPattern.LinkType;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.matchers.method.MethodMatchers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import java.util.regex.Pattern;

@AutoService(BugChecker.class)
@BugPattern(
name = "LogSafePreconditionsMessageFormat",
category = Category.ONE_OFF,
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = LinkType.CUSTOM,
severity = SeverityLevel.ERROR,
summary = "logsafe Preconditions.checkX() methods should not have print-f style formatting.")
public final class LogSafePreconditionsMessageFormat extends PreconditionsMessageFormat {

private static final long serialVersionUID = 1L;

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

public LogSafePreconditionsMessageFormat() {
super(LOGSAFE_PRECONDITIONS_METHOD);
}

@Override
protected Description matchMessageFormat(MethodInvocationTree tree, String message, VisitorState state) {
if (!message.contains("%s")) {
return Description.NO_MATCH;
}

return buildDescription(tree).setMessage(
"Do not use printf-style formatting in logsafe Preconditions.").build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
@AutoService(BugChecker.class)
@BugPattern(
name = "PreconditionsConstantMessage",
category = Category.GUAVA,
category = Category.ONE_OFF,
severity = SeverityLevel.ERROR,
summary = "Allow only constant messages to Preconditions.checkX() methods")
public final class PreconditionsConstantMessage extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {
Expand All @@ -45,7 +45,7 @@ public final class PreconditionsConstantMessage extends BugChecker implements Bu
private static final Matcher<ExpressionTree> PRECONDITIONS_METHOD =
Matchers.anyOf(
MethodMatchers.staticMethod()
.onClass("com.google.common.base.Preconditions")
.onClassAny("com.google.common.base.Preconditions", "com.palantir.logsafe.Preconditions")
.withNameMatching(Pattern.compile("checkArgument|checkState|checkNotNull")));

private final Matcher<ExpressionTree> compileTimeConstExpressionMatcher =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* (c) Copyright 2017 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.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LiteralTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import java.util.List;

abstract class PreconditionsMessageFormat extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {

private final Matcher<ExpressionTree> methodMatcher;

protected PreconditionsMessageFormat(Matcher<ExpressionTree> methodMatcher) {
this.methodMatcher = methodMatcher;
}

protected abstract Description matchMessageFormat(MethodInvocationTree tree, String message, VisitorState state);

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!methodMatcher.matches(tree, state)) {
return Description.NO_MATCH;
}

List<? extends ExpressionTree> args = tree.getArguments();
if (args.size() <= 1) {
return Description.NO_MATCH;
}

ExpressionTree messageArg = args.get(1);
if (!messageArg.getKind().equals(Tree.Kind.STRING_LITERAL)) {
return Description.NO_MATCH;
}

String message;
try {
message = (String) ((LiteralTree) messageArg).getValue();
} catch (ClassCastException exception) {
return Description.NO_MATCH;
}

return matchMessageFormat(tree, message, state);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* (c) Copyright 2018 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.errorprone.CompilationTestHelper;
import org.junit.Before;
import org.junit.Test;

public final class GuavaPreconditionsMessageFormatTests extends PreconditionsTests {
private CompilationTestHelper compilationHelper;

@Before
public void before() {
compilationHelper = CompilationTestHelper.newInstance(GuavaPreconditionsMessageFormat.class, getClass());
}

@Override
public CompilationTestHelper compilationHelper() {
return compilationHelper;
}

@Test
public void positive() throws Exception {
String diagnostic = "Use printf-style formatting";
failGuava(diagnostic, "Preconditions.checkArgument(param != \"string\", \"message {}\", 123L);");
failGuava(diagnostic, "Preconditions.checkState(param != \"string\", \"message {}\", 123L);");
failGuava(diagnostic, "Preconditions.checkNotNull(param, \"message {}\", 123L);");

failGuava(diagnostic, "Preconditions.checkArgument(param != \"string\", \"message {} {}\", 'a', 'b');");
failGuava(diagnostic, "Preconditions.checkState(param != \"string\", \"message {} {}\", 'a', 'b');");
failGuava(diagnostic, "Preconditions.checkNotNull(param, \"message {} {}\", 'a', 'b');");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* (c) Copyright 2018 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.errorprone.CompilationTestHelper;
import org.junit.Before;
import org.junit.Test;

public final class LogSafePreconditionsMessageFormatTests extends PreconditionsTests {
private CompilationTestHelper compilationHelper;

@Before
public void before() {
compilationHelper = CompilationTestHelper.newInstance(LogSafePreconditionsMessageFormat.class, getClass());
}

@Override
public CompilationTestHelper compilationHelper() {
return compilationHelper;
}

@Test
public void positive() throws Exception {
String diagnostic = "Do not use printf-style formatting";
failLogSafe(diagnostic, "Preconditions.checkArgument(param != \"string\", \"message %s\","
+ " UnsafeArg.of(\"long\", 123L));");
failLogSafe(diagnostic, "Preconditions.checkState(param != \"string\", \"message %s\","
+ " UnsafeArg.of(\"long\", 123L));");
failLogSafe(diagnostic, "Preconditions.checkNotNull(param, \"message %s\","
+ " UnsafeArg.of(\"long\", 123L));");

failLogSafe(diagnostic, "Preconditions.checkArgument(param != \"string\", \"message %s %s\","
+ " UnsafeArg.of(\"char1\", 'a'), UnsafeArg.of(\"char2\", 'b'));");
failLogSafe(diagnostic, "Preconditions.checkState(param != \"string\", \"message %s %s\","
+ " UnsafeArg.of(\"char1\", 'a'), UnsafeArg.of(\"char2\", 'b'));");
failLogSafe(diagnostic, "Preconditions.checkNotNull(param, \"message %s %s\","
+ " UnsafeArg.of(\"char1\", 'a'), UnsafeArg.of(\"char2\", 'b'));");
}
}
Loading

0 comments on commit 628aa7c

Please sign in to comment.