Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -186,6 +186,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
- `UnsafeGaugeRegistration`: Use TaggedMetricRegistry.registerWithReplacement over TaggedMetricRegistry.gauge.
- `BracesRequired`: Require braces for loops and if expressions.
- `CollectionStreamForEach`: Collection.forEach is more efficient than Collection.stream().forEach.
- `LoggerEnclosingClass`: Loggers created using getLogger(Class<?>) must reference their enclosing class.

### Programmatic Application

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* (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.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.fixes.SuggestedFix;
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.google.errorprone.matchers.method.MethodMatchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Symbol;

@AutoService(BugChecker.class)
@BugPattern(
name = "LoggerEnclosingClass",
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = BugPattern.LinkType.CUSTOM,
providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION,
severity = BugPattern.SeverityLevel.WARNING,
summary = "Loggers created using getLogger(Class<?>) must reference their enclosing class.")
public final class LoggerEnclosingClass extends BugChecker implements BugChecker.VariableTreeMatcher {

private static final Matcher<ExpressionTree> getLoggerMatcher = MethodMatchers.staticMethod()
.onClass("org.slf4j.LoggerFactory")
.named("getLogger")
// Only match the 'class' constructor
.withParameters(Class.class.getName());

private static final Matcher<VariableTree> fieldMatcher = Matchers.isField();

@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
if (!fieldMatcher.matches(tree, state)) {
return Description.NO_MATCH;
}
ExpressionTree initializer = tree.getInitializer();
if (initializer == null) {
return Description.NO_MATCH;
}
if (!getLoggerMatcher.matches(initializer, state)) {
return Description.NO_MATCH;
}

MethodInvocationTree getLoggerInvocation = (MethodInvocationTree) initializer;
ExpressionTree classArgument = getLoggerInvocation.getArguments().get(0);

if (!(classArgument instanceof MemberSelectTree)) {
return Description.NO_MATCH;
}
MemberSelectTree memberSelectTree = (MemberSelectTree) classArgument;
if (!memberSelectTree.getIdentifier().contentEquals("class")
|| memberSelectTree.getExpression().getKind() != Tree.Kind.IDENTIFIER) {
return Description.NO_MATCH;
}

Symbol targetSymbol = ASTHelpers.getSymbol(memberSelectTree.getExpression());
Symbol.ClassSymbol enclosingClassSymbol = ASTHelpers.enclosingClass(ASTHelpers.getSymbol(tree));
if (targetSymbol == null || enclosingClassSymbol == null) {
return Description.NO_MATCH;
}
if (state.getTypes().isSameType(targetSymbol.type, enclosingClassSymbol.type)) {
return Description.NO_MATCH;
}
SuggestedFix.Builder fix = SuggestedFix.builder();
return buildDescription(classArgument)
.addFix(fix.replace(
classArgument,
SuggestedFixes.prettyType(state, fix, enclosingClassSymbol.type) + ".class")
.build())
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* (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 LoggerEnclosingClassTest {

@Test
void testFix() {
fix().addInputLines(
"Test.java",
"import org.slf4j.*;",
"class Test {",
" private static final Logger log = LoggerFactory.getLogger(String.class);",
"}")
.addOutputLines(
"Test.java",
"import org.slf4j.*;",
"class Test {",
" private static final Logger log = LoggerFactory.getLogger(Test.class);",
"}")
.doTest();
}

@Test
void testNegative() {
fix().addInputLines(
"Test.java",
"import org.slf4j.*;",
"class Test {",
// Not great, but it's out of scope for this check to validate dynamic cases.
" private static final Logger log = LoggerFactory.getLogger(dynamic());",
" private static Class<String> dynamic() {",
" return String.class;",
" }",
" private static void func() {",
" LoggerFactory.getLogger(String.class);",
" Logger local = LoggerFactory.getLogger(String.class);",
" }",
"}")
.expectUnchanged()
.doTest();
}

private RefactoringValidator fix() {
return RefactoringValidator.of(new LoggerEnclosingClass(), getClass());
}
}
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-1163.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: improvement
improvement:
description: Add error prone LoggerEnclosingClass check. Loggers created using getLogger(Class<?>)
must reference their enclosing class.
links:
- https://github.com/palantir/gradle-baseline/pull/1163
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class BaselineErrorProneExtension {
"ExecutorSubmitRunnableFutureIgnored",
"FinalClass",
"LambdaMethodReference",
"LoggerEnclosingClass",
"OptionalOrElseMethodInvocation",
"PreferBuiltInConcurrentKeySet",
"PreferCollectionTransform",
Expand Down