Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
- `LoggerEnclosingClass`: Loggers created using getLogger(Class<?>) must reference their enclosing class.
- `UnnecessaryLambdaArgumentParentheses`: Lambdas with a single parameter do not require argument parentheses.
- `RawTypes`: Avoid raw types; add appropriate type parameters if possible.
- `VisibleForTestingPackagePrivate`: `@VisibleForTesting` members should be package-private.

### Programmatic Application

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* (c) Copyright 2020 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.common.annotations.VisibleForTesting;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import javax.lang.model.element.Modifier;

/** Development Practices: Writing good unit tests. */
@AutoService(BugChecker.class)
@BugPattern(
name = "VisibleForTestingPackagePrivate",
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = BugPattern.LinkType.CUSTOM,
severity = BugPattern.SeverityLevel.WARNING,
summary = "@VisibleForTesting members should be package-private.")
public final class VisibleForTestingPackagePrivate extends BugChecker
implements BugChecker.ClassTreeMatcher, BugChecker.MethodTreeMatcher, BugChecker.VariableTreeMatcher {

private static final Matcher<Tree> matcher = Matchers.allOf(
Matchers.hasAnnotation(VisibleForTesting.class),
Matchers.anyOf(
MoreMatchers.hasExplicitModifier(Modifier.PROTECTED),
MoreMatchers.hasExplicitModifier(Modifier.PUBLIC)));

private Description match(Tree tree, VisitorState state) {
if (matcher.matches(tree, state)) {
return buildDescription(tree)
// This may break code that references the visible component, so it should not
// be applied by default.
.addFix(SuggestedFixes.removeModifiers(tree, state, Modifier.PROTECTED, Modifier.PUBLIC))
.build();
}
return Description.NO_MATCH;
}

@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
return match(tree, state);
}

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
return match(tree, state);
}

@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
return match(tree, state);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/*
* (c) Copyright 2020 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 org.junit.jupiter.api.Test;

class VisibleForTestingPackagePrivateTest {

@Test
void testMethod() {
fix().addInputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public class Test {",
" @VisibleForTesting",
" public void foo() {}",
" @VisibleForTesting",
" public static void staticFoo() {}",
" @VisibleForTesting",
" protected void bar() {}",
" public void baz() {}",
" public static void staticBaz() {}",
" protected void bang() {}",
"}")
.addOutputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public class Test {",
" @VisibleForTesting",
" void foo() {}",
" @VisibleForTesting",
" static void staticFoo() {}",
" @VisibleForTesting",
" void bar() {}",
" public void baz() {}",
" public static void staticBaz() {}",
" protected void bang() {}",
"}")
.doTest();
}

@Test
void testField() {
fix().addInputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public class Test {",
" @VisibleForTesting",
" public int foo = 1;",
" public int bar = 1;",
" @VisibleForTesting",
" public static final int FOO = 1;",
" public static final int BAR = 1;",
"}")
.addOutputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public class Test {",
" @VisibleForTesting",
" int foo = 1;",
" public int bar = 1;",
" @VisibleForTesting",
" static final int FOO = 1;",
" public static final int BAR = 1;",
"}")
.doTest();
}

@Test
void testType() {
fix().addInputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public class Test {",
" @VisibleForTesting",
" public static final class Foo {}",
" public static final class Bar {}",
"}")
.addOutputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public class Test {",
" @VisibleForTesting",
" static final class Foo {}",
" public static final class Bar {}",
"}")
.doTest();
}

@Test
void testNegativeInterfaceMethods() {
fix().addInputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public interface Test {",
" @VisibleForTesting",
" default void foo() {}",
" @VisibleForTesting",
" static void staticFoo() {}",
"}")
.expectUnchanged()
.doTest();
}

@Test
void testNegativeInterfaceFields() {
fix().addInputLines(
"Test.java",
"import com.google.common.annotations.VisibleForTesting;",
"public interface Test {",
" @VisibleForTesting",
" int FOO = 5;",
"}")
.expectUnchanged()
.doTest();
}

private RefactoringValidator fix() {
return RefactoringValidator.of(new VisibleForTestingPackagePrivate(), getClass());
}
}
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1253.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Migrate VisibleForTesting validation from checkstyle to errorprone
links:
- https://github.com/palantir/gradle-baseline/pull/1253
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@
<module name="NewlineAtEndOfFile"> <!-- Java Style Guide: Line ending: LF -->
<property name="lineSeparator" value="lf"/>
</module>
<module name="RegexpMultiline"> <!-- Development Practices: Writing good unit tests -->
<property name="fileExtensions" value="java"/>
<property name="format" value="@VisibleForTesting\s+(protected|public)"/>
<property name="message" value="@VisibleForTesting members should be package-private."/>
</module>
<module name="RegexpSingleline"> <!-- No reference needed as this is evident. -->
<property name="format" value="&lt;&lt;&lt;&lt;&lt;&lt;&lt;"/>
<property name="message" value="Found (&lt;&lt;&lt;&lt;&lt;&lt;&lt;), so it looks like you had a merge conflict that compiles. Please fix it."/>
Expand Down