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.

Sign up for GitHub

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

Jump to bottom

Separate positive test cases #702

Merged
merged 4 commits into from
Jul 22, 2019
Merged

Separate positive test cases #702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
863aeae
Adding incorrect positive case does not cause tests to fail
pkoenig10 Jul 18, 2019
e21286d
Revert "Adding incorrect positive case does not cause tests to fail"
pkoenig10 Jul 18, 2019
b848a21
Separate positive test cases
pkoenig10 Jul 18, 2019
5a7baf3
Changelog
pkoenig10 Jul 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions ...-prone/src/main/java/com/palantir/baseline/errorprone/OptionalOrElseMethodInvocation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.google.errorprone.matchers.method.MethodMatchers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;

@AutoService(BugChecker.class)
@BugPattern(
Expand All @@ -46,8 +45,13 @@ public final class OptionalOrElseMethodInvocation extends BugChecker implements
.onExactClass("java.util.Optional")
.named("orElse");

private static final Matcher<Tree> METHOD_INVOCATIONS =
Matchers.contains(ExpressionTree.class, MethodMatchers.anyMethod());
private static final Matcher<ExpressionTree> METHOD_OR_CONSTRUCTOR = Matchers.anyOf(
MethodMatchers.anyMethod(),
MethodMatchers.constructor());

private static final Matcher<ExpressionTree> METHOD_INVOCATIONS = Matchers.anyOf(
METHOD_OR_CONSTRUCTOR,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required because while the symbol for a method invocation is a MethodSymbol, the symbol for a constructor invocation is just a TypeSymbol. Thus Matchers.contains(...) won't detect a expression that is just a constructor invocation.

Matchers.contains(ExpressionTree.class, METHOD_OR_CONSTRUCTOR));

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
Expand Down
14 changes: 13 additions & 1 deletion ...rror-prone/src/test/java/com/palantir/baseline/errorprone/CatchBlockLogExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,22 @@ public void before() {
}

@Test
public void testLogException() {
public void testLogIllegalArgumentException() {
test(IllegalArgumentException.class, "log.info(\"hello\", e);", Optional.empty());
}

@Test
public void testLogRuntimeException() {
test(RuntimeException.class, "log.info(\"hello\", e);", Optional.empty());
}

@Test
public void testLogException() {
test(Exception.class, "log.info(\"hello\", e);", Optional.empty());
}

@Test
public void testLogThrowable() {
test(Throwable.class, "log.info(\"hello\", e);", Optional.empty());
}

Expand Down
46 changes: 39 additions & 7 deletions ...prone/src/test/java/com/palantir/baseline/errorprone/DangerousJsonTypeInfoUsageTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,56 @@ public void before() {
}

@Test
public void testMustNotUseClassVariants() throws Exception {
public void testClass() {
positive("JsonTypeInfo.Id.CLASS");
positive("JsonTypeInfo.Id.MINIMAL_CLASS");
}

@Test
public void testClass_fullyQualified() {
positive("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CLASS");
}

@Test
public void testMinimalClass() {
positive("JsonTypeInfo.Id.MINIMAL_CLASS");
}

@Test
public void testMinimalClass_fullyQualified() {
positive("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.MINIMAL_CLASS");
}

@Test
public void testMayUseNoneNameCustomVariants() throws Exception {
public void testNone() {
negative("JsonTypeInfo.Id.NONE");
negative("JsonTypeInfo.Id.NAME");
negative("JsonTypeInfo.Id.CUSTOM");
}

@Test
public void testNone_fullyQualified() {
negative("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NONE");
}

@Test
public void testName() {
negative("JsonTypeInfo.Id.NAME");
}

@Test
public void testName_fullyQualified() {
negative("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NAME");
}

@Test
public void testCustom() {
negative("JsonTypeInfo.Id.CUSTOM");
}

@Test
public void testCustom_fullyQualified() {
negative("com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CUSTOM");
}

private void positive(String variant) throws Exception {
private void positive(String variant) {
compilationHelper.addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
Expand All @@ -57,7 +89,7 @@ private void positive(String variant) throws Exception {
).doTest();
}

private void negative(String variant) throws Exception {
private void negative(String variant) {
compilationHelper.addSourceLines(
"Bean.java",
"import com.fasterxml.jackson.annotation.JsonTypeInfo;",
Expand Down
39 changes: 30 additions & 9 deletions ...ne/PreconditionsConstantMessageTests.java → ...avaPreconditionsConstantMessageTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import org.junit.Before;
import org.junit.Test;

public final class PreconditionsConstantMessageTests extends PreconditionsTests {
public final class GuavaPreconditionsConstantMessageTests extends PreconditionsTests {

private static final String DIAGNOSTIC = "non-constant message";

private CompilationTestHelper compilationHelper;

Expand All @@ -35,15 +37,34 @@ public CompilationTestHelper compilationHelper() {
}

@Test
public void positive() throws Exception {
String diagnostic = "non-constant message";
failGuava(diagnostic, "Preconditions.checkArgument(param != \"string\", \"constant\" + param);");
failGuava(diagnostic, "Preconditions.checkState(param != \"string\", \"constant\" + param);");
failGuava(diagnostic, "Preconditions.checkNotNull(param, \"constant\" + param);");
public void testCheckArgument_stringConcatenate() {
failGuava(DIAGNOSTIC, "Preconditions.checkArgument(param != \"string\", \"constant\" + param);");
}

failGuava(diagnostic,
@Test
public void testCheckArgument_stringFormat() {
failGuava(DIAGNOSTIC,
"Preconditions.checkArgument(param != \"string\", String.format(\"constant %s\", param));");
failGuava(diagnostic, "Preconditions.checkState(param != \"string\", String.format(\"constant %s\", param));");
failGuava(diagnostic, "Preconditions.checkNotNull(param, String.format(\"constant %s\", param));");
}

@Test
public void testCheckState_stringConcatenate() {
failGuava(DIAGNOSTIC, "Preconditions.checkState(param != \"string\", \"constant\" + param);");
}

@Test
public void testCheckState_stringFormat() {
failGuava(DIAGNOSTIC,
"Preconditions.checkState(param != \"string\", String.format(\"constant %s\", param));");
}

@Test
public void testCheckNotNull_stringConcatenate() {
failGuava(DIAGNOSTIC, "Preconditions.checkNotNull(param, \"constant\" + param);");
}

@Test
public void testCheckNotNull_stringFormat() {
failGuava(DIAGNOSTIC, "Preconditions.checkNotNull(param, String.format(\"constant %s\", param));");
}
}
39 changes: 30 additions & 9 deletions .../src/test/java/com/palantir/baseline/errorprone/GuavaPreconditionsMessageFormatTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import org.junit.Test;

public final class GuavaPreconditionsMessageFormatTests extends PreconditionsTests {

private static final String DIAGNOSTIC = "Use printf-style formatting";

private CompilationTestHelper compilationHelper;

@Before
Expand All @@ -34,14 +37,32 @@ public CompilationTestHelper 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');");
public void testCheckArgument() {
failGuava(DIAGNOSTIC, "Preconditions.checkArgument(param != \"string\", \"message {}\", 123L);");
}

@Test
public void testCheckArgument_multipleArgs() {
failGuava(DIAGNOSTIC, "Preconditions.checkArgument(param != \"string\", \"message {} {}\", 'a', 'b');");
}

@Test
public void testCheckState() {
failGuava(DIAGNOSTIC, "Preconditions.checkState(param != \"string\", \"message {}\", 123L);");
}

@Test
public void testCheckState_multipleArgs() {
failGuava(DIAGNOSTIC, "Preconditions.checkState(param != \"string\", \"message {} {}\", 'a', 'b');");
}

@Test
public void testCheckNotNull() {
failGuava(DIAGNOSTIC, "Preconditions.checkNotNull(param, \"message {}\", 123L);");
}

@Test
public void testCheckNotNull_multipleArgs() {
failGuava(DIAGNOSTIC, "Preconditions.checkNotNull(param, \"message {} {}\", 'a', 'b');");
}
}
37 changes: 28 additions & 9 deletions .../test/java/com/palantir/baseline/errorprone/LogSafePreconditionsConstantMessageTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

public final class LogSafePreconditionsConstantMessageTests extends PreconditionsTests {

private static final String DIAGNOSTIC = "non-constant message";

private CompilationTestHelper compilationHelper;

@Before
Expand All @@ -35,17 +37,34 @@ public CompilationTestHelper compilationHelper() {
}

@Test
public void positive() throws Exception {
String diagnostic = "non-constant message";

failLogSafe(diagnostic, "Preconditions.checkArgument(param != \"string\", \"constant\" + param);");
failLogSafe(diagnostic, "Preconditions.checkState(param != \"string\", \"constant\" + param);");
failLogSafe(diagnostic, "Preconditions.checkNotNull(param, \"constant\" + param);");
public void testCheckArgument_stringConcatenate() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkArgument(param != \"string\", \"constant\" + param);");
}

failLogSafe(diagnostic,
@Test
public void testCheckArgument_stringFormat() {
failLogSafe(DIAGNOSTIC,
"Preconditions.checkArgument(param != \"string\", String.format(\"constant %s\", param));");
failLogSafe(diagnostic,
}

@Test
public void testCheckState_stringConcatenate() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkState(param != \"string\", \"constant\" + param);");
}

@Test
public void testCheckState_stringFormat() {
failLogSafe(DIAGNOSTIC,
"Preconditions.checkState(param != \"string\", String.format(\"constant %s\", param));");
failLogSafe(diagnostic, "Preconditions.checkNotNull(param, String.format(\"constant %s\", param));");
}

@Test
public void testCheckNotNull_stringConcatenate() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkNotNull(param, \"constant\" + param);");
}

@Test
public void testCheckNotNull_stringFormat() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkNotNull(param, String.format(\"constant %s\", param));");
}
}
41 changes: 31 additions & 10 deletions ...rc/test/java/com/palantir/baseline/errorprone/LogSafePreconditionsMessageFormatTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import org.junit.Test;

public final class LogSafePreconditionsMessageFormatTests extends PreconditionsTests {

private static final String DIAGNOSTIC = "Do not use printf-style formatting";

private CompilationTestHelper compilationHelper;

@Before
Expand All @@ -34,20 +37,38 @@ public CompilationTestHelper 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\","
public void testCheckArgument() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkArgument(param != \"string\", \"message %s\","
+ " UnsafeArg.of(\"long\", 123L));");
}

failLogSafe(diagnostic, "Preconditions.checkArgument(param != \"string\", \"message %s %s\","
@Test
public void testCheckArgument_multipleArgs() {
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\","
}

@Test
public void testCheckState() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkState(param != \"string\", \"message %s\","
+ " UnsafeArg.of(\"long\", 123L));");
}

@Test
public void testCheckState_multipleArgs() {
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\","
}

@Test
public void testCheckNotNull() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkNotNull(param, \"message %s\","
+ " UnsafeArg.of(\"long\", 123L));");
}

@Test
public void testCheckNotNull_multipleArgs() {
failLogSafe(DIAGNOSTIC, "Preconditions.checkNotNull(param, \"message %s %s\","
+ " UnsafeArg.of(\"char1\", 'a'), UnsafeArg.of(\"char2\", 'b'));");
}
}
Loading