Skip to content

New error-prone rule: AutoCloseableMustBeClosed #1673

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

Merged
merged 4 commits into from
Mar 6, 2021
Merged

New error-prone rule: AutoCloseableMustBeClosed #1673

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
857ed0e
Add AutoCloseableMustBeClosed suggested bug checker
schlosna Mar 5, 2021
80ced69
AutoCloseableMustBeClosed ignores Streams
schlosna Mar 5, 2021
814633b
address review comments
schlosna Mar 5, 2021
27ecd00
Add generated changelog entries
schlosna Mar 5, 2021
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
83 changes: 83 additions & 0 deletions ...error-prone/src/main/java/com/palantir/baseline/errorprone/AutoCloseableMustBeClosed.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* (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.bugpatterns.BugChecker.MethodTreeMatcher;
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.sun.source.tree.MethodTree;
import java.util.stream.BaseStream;

@AutoService(BugChecker.class)
@BugPattern(
name = "AutoCloseableMustBeClosed",
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = BugPattern.LinkType.CUSTOM,
providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION,
severity = SeverityLevel.SUGGESTION,
summary = AutoCloseableMustBeClosed.SUMMARY)
public final class AutoCloseableMustBeClosed extends BugChecker implements MethodTreeMatcher {

static final String SUMMARY = "If a constructor or method returns an AutoCloseable, "
+ "it should be annotated @MustBeClosed to ensure callers appropriately close resources";
private static final String MUST_BE_CLOSED_TYPE = "com.google.errorprone.annotations.MustBeClosed";
private static final String CAN_IGNORE_RETURN_VALUE_TYPE = "com.google.errorprone.annotations.CanIgnoreReturnValue";

private static final Matcher<MethodTree> methodReturnsAutoCloseable = Matchers.allOf(
Matchers.not(Matchers.methodIsConstructor()),
Matchers.methodReturns(Matchers.isSubtypeOf(AutoCloseable.class)),
// ignore Stream for now, see https://errorprone.info/bugpattern/StreamResourceLeak
Matchers.not(Matchers.methodReturns(Matchers.isSubtypeOf(BaseStream.class))));

private static final Matcher<MethodTree> constructsAutoCloseable = Matchers.allOf(
Matchers.methodIsConstructor(),
Matchers.enclosingClass(Matchers.isSubtypeOf(AutoCloseable.class)),
// ignore Stream for now, see https://errorprone.info/bugpattern/StreamResourceLeak
Matchers.not(Matchers.enclosingClass(Matchers.isSubtypeOf(BaseStream.class))));

private static final Matcher<MethodTree> methodNotAnnotatedMustBeClosed =
Matchers.not(Matchers.hasAnnotation(MUST_BE_CLOSED_TYPE));

private static final Matcher<MethodTree> methodNotAnnotatedIgnoreReturnValue =
Matchers.not(Matchers.hasAnnotation(CAN_IGNORE_RETURN_VALUE_TYPE));

private static final Matcher<MethodTree> methodShouldBeAnnotatedMustBeClosed = Matchers.allOf(
Matchers.anyOf(methodReturnsAutoCloseable, constructsAutoCloseable),
methodNotAnnotatedMustBeClosed,
methodNotAnnotatedIgnoreReturnValue);

@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (methodShouldBeAnnotatedMustBeClosed.matches(tree, state)) {
SuggestedFix.Builder builder = SuggestedFix.builder().addImport(MUST_BE_CLOSED_TYPE);
String annotation = SuggestedFixes.qualifyType(state, builder, MUST_BE_CLOSED_TYPE);
return buildDescription(tree)
.setMessage(SUMMARY)
.addFix(builder.prefixWith(tree, "@" + annotation + " ").build())
.build();
}
return Description.NO_MATCH;
}
}
218 changes: 218 additions & 0 deletions ...r-prone/src/test/java/com/palantir/baseline/errorprone/AutoCloseableMustBeClosedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
/*
* (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.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public final class AutoCloseableMustBeClosedTest {

private CompilationTestHelper compilationHelper;
private RefactoringValidator refactoringTestHelper;

@BeforeEach
public void before() {
compilationHelper = CompilationTestHelper.newInstance(AutoCloseableMustBeClosed.class, getClass());
refactoringTestHelper = RefactoringValidator.of(new AutoCloseableMustBeClosed(), getClass());
}

@Test
public void testAutoCloseableReturn() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.io.*;",
"class Test {",
" // BUG: Diagnostic contains: should be annotated @MustBeClosed",
" private AutoCloseable autoCloseable() {",
" return new ByteArrayInputStream(new byte[0]);",
" }",
"}")
.doTest();
}

@Test
public void testInputStream() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.io.*;",
"class Test {",
" // BUG: Diagnostic contains: should be annotated @MustBeClosed",
" private InputStream inputStream() {",
" return new ByteArrayInputStream(new byte[0]);",
" }",
"}")
.doTest();
}

@Test
public void testOutputStream() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.io.*;",
"class Test {",
" // BUG: Diagnostic contains: should be annotated @MustBeClosed",
" private OutputStream outputStream() {",
" return new ByteArrayOutputStream();",
" }",
"}")
.doTest();
}

@Test
public void testConstructor() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.io.*;",
"class Test extends FilterOutputStream {",
" // BUG: Diagnostic contains: should be annotated @MustBeClosed",
" public Test() {",
" super(new ByteArrayOutputStream());",
" }",
"}")
.doTest();
}

@Test
public void testIgnoreStream() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.*;",
"class Test {",
" private Stream<Integer> a() {",
" return Stream.of(1);",
" }",
"",
" private IntStream b() {",
" return IntStream.of(1);",
" }",
"}")
.doTest();
}

@Test
public void testAlreadyAnnotated() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.*;",
"import java.io.*;",
"class Test {",
" @MustBeClosed",
" private Writer alreadyAnnotated() {",
" return new StringWriter();",
" }",
"}")
.doTest();
}

@Test
public void testCanIgnoreReturnValue() {
compilationHelper
.addSourceLines(
"Test.java",
"import com.google.errorprone.annotations.*;",
"import java.io.*;",
"class Test {",
" @CanIgnoreReturnValue",
" private Closeable ignoreReturn() {",
" return new StringReader(\"\");",
" }",
"}")
.doTest();
}

@Test
public void testSuggestedFixAddMustBeClosed() {
refactoringTestHelper
.addInputLines(
"Test.java",
"import com.google.errorprone.annotations.CanIgnoreReturnValue;",
"import java.io.*;",
"class Test extends FilterOutputStream {",
"",
" public Test() {",
" super(new ByteArrayOutputStream());",
" }",
"",
" public static Test create() {",
" return new Test();",
" }",
"",
" private AutoCloseable autoCloseable() {",
" return new ByteArrayInputStream(new byte[0]);",
" }",
"",
" private InputStream inputStream() {",
" return new ByteArrayInputStream(new byte[0]);",
" }",
"",
" private OutputStream outputStream() {",
" return new ByteArrayOutputStream();",
" }",
"",
" @CanIgnoreReturnValue",
" private Closeable ignoreReturn() {",
" return new StringReader(\"\");",
" }",
"}")
.addOutputLines(
"Test.java",
"import com.google.errorprone.annotations.CanIgnoreReturnValue;",
"import com.google.errorprone.annotations.MustBeClosed;",
"import java.io.*;",
"class Test extends FilterOutputStream {",
"",
" @MustBeClosed",
" public Test() {",
" super(new ByteArrayOutputStream());",
" }",
"",
" @MustBeClosed",
" public static Test create() {",
" return new Test();",
" }",
"",
" @MustBeClosed",
" private AutoCloseable autoCloseable() {",
" return new ByteArrayInputStream(new byte[0]);",
" }",
"",
" @MustBeClosed",
" private InputStream inputStream() {",
" return new ByteArrayInputStream(new byte[0]);",
" }",
"",
" @MustBeClosed",
" private OutputStream outputStream() {",
" return new ByteArrayOutputStream();",
" }",
"",
" @CanIgnoreReturnValue",
" private Closeable ignoreReturn() {",
" return new StringReader(\"\");",
" }",
"}")
.doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
}
}