From 72f91850bd20e4ed7955495859b4f7ebf00e7dfe Mon Sep 17 00:00:00 2001 From: Christian Briones Date: Mon, 17 Aug 2020 16:52:51 -0700 Subject: [PATCH] Allow private methods to inherit default annotations from package or class scope (#1222) * refs #374: reproduce reported problem * allow private methods to inherit default annotations Remove the restriction that a method must not be private in order to inherit default annotations from package or class scope. Resolves spotbugs/spotbugs#374. * bump year in license header * add a comment in changelog as per the PR template * ./gradlew spotlessApply * address pr comments - fix copyright attribution - move entry in changelog Co-authored-by: Ritee --- CHANGELOG.md | 1 + .../umd/cs/findbugs/detect/Issue374Test.java | 46 +++++++++++++++++++ .../ba/jsr305/TypeQualifierApplications.java | 5 -- .../java/ghIssues/issue374/ClassLevel.java | 19 ++++++++ .../java/ghIssues/issue374/MethodLevel.java | 19 ++++++++ .../java/ghIssues/issue374/PackageLevel.java | 17 +++++++ .../java/ghIssues/issue374/package-info.java | 5 ++ 7 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue374Test.java create mode 100644 spotbugsTestCases/src/java/ghIssues/issue374/ClassLevel.java create mode 100644 spotbugsTestCases/src/java/ghIssues/issue374/MethodLevel.java create mode 100644 spotbugsTestCases/src/java/ghIssues/issue374/PackageLevel.java create mode 100644 spotbugsTestCases/src/java/ghIssues/issue374/package-info.java diff --git a/CHANGELOG.md b/CHANGELOG.md index c8dec6209b7..a0409623f95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. ### Fixed * [A meaningless exception data from `SAXBugCollectionHandler`](https://lgtm.com/projects/g/spotbugs/spotbugs/rev/a77ab08634687b7791e902636996ab6184462693) * Use URI for files instead of converting string to URI each time. Fixes tests on Windows. +* Allow private methods to inherit default annotations from package or class scope. ([#374](https://github.com/spotbugs/spotbugs/issues/374)) ### Added * Implement [issue 390](https://github.com/spotbugs/spotbugs/issues/390) as a detector, `DontAssertInstanceofInTests`, which reports bugs of type `JUA_DONT_ASSERT_INSTANCEOF_IN_TESTS`. diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue374Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue374Test.java new file mode 100644 index 00000000000..c78c68b3999 --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue374Test.java @@ -0,0 +1,46 @@ +/* + * Contributions to SpotBugs + * Copyright (C) 2020, ritchan + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +package edu.umd.cs.findbugs.detect; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; +import org.junit.Test; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.junit.Assert.assertThat; + + +/** + * @see GitHub issue + */ +public class Issue374Test extends AbstractIntegrationTest { + @Test + public void test() { + performAnalysis( + "ghIssues/issue374/package-info.class", + "ghIssues/issue374/ClassLevel.class", + "ghIssues/issue374/MethodLevel.class", + "ghIssues/issue374/PackageLevel.class"); + BugInstanceMatcher matcher = new BugInstanceMatcherBuilder() + .bugType("NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE") + .build(); + assertThat(getBugCollection(), containsExactly(3, matcher)); + } +} diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/jsr305/TypeQualifierApplications.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/jsr305/TypeQualifierApplications.java index 2b070178206..7246ced2389 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/jsr305/TypeQualifierApplications.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/ba/jsr305/TypeQualifierApplications.java @@ -986,11 +986,6 @@ private static TypeQualifierAnnotation computeEffectiveTypeQualifierAnnotation(T // default annotations } - /** private methods don't inherit from class or package scope */ - if (xmethod.isPrivate()) { - stopAtMethodScope = true; - } - boolean stopAtClassScope = false; if (!xmethod.isPublic() && !xmethod.isProtected() && (xmethod.isStatic() || "".equals(xmethod.getName()))) { diff --git a/spotbugsTestCases/src/java/ghIssues/issue374/ClassLevel.java b/spotbugsTestCases/src/java/ghIssues/issue374/ClassLevel.java new file mode 100644 index 00000000000..f79b593ba77 --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue374/ClassLevel.java @@ -0,0 +1,19 @@ +package ghIssues.issue374; + +import javax.annotation.Nullable; +import javax.annotation.ParametersAreNonnullByDefault; + +@ParametersAreNonnullByDefault +public class ClassLevel { + public String method() { + return methodNullable(null); + } + + private String methodNullable(@Nullable final String test) { + return methodNonNull(test); + } + + private String methodNonNull(final String test) { + return test; + } +} diff --git a/spotbugsTestCases/src/java/ghIssues/issue374/MethodLevel.java b/spotbugsTestCases/src/java/ghIssues/issue374/MethodLevel.java new file mode 100644 index 00000000000..8cbcb8080b0 --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue374/MethodLevel.java @@ -0,0 +1,19 @@ +package ghIssues.issue374; + +import javax.annotation.Nullable; +import javax.annotation.ParametersAreNonnullByDefault; + +public class MethodLevel { + public String method() { + return methodNullable(null); + } + + private String methodNullable(@Nullable final String test) { + return methodNonNull(test); + } + + @ParametersAreNonnullByDefault + private String methodNonNull(final String test) { + return test; + } +} diff --git a/spotbugsTestCases/src/java/ghIssues/issue374/PackageLevel.java b/spotbugsTestCases/src/java/ghIssues/issue374/PackageLevel.java new file mode 100644 index 00000000000..220a147ee27 --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue374/PackageLevel.java @@ -0,0 +1,17 @@ +package ghIssues.issue374; + +import javax.annotation.Nullable; + +public class PackageLevel { + public String method() { + return methodNullable(null); + } + + private String methodNullable(@Nullable final String test) { + return methodNonNull(test); + } + + private String methodNonNull(final String test) { + return test; + } +} diff --git a/spotbugsTestCases/src/java/ghIssues/issue374/package-info.java b/spotbugsTestCases/src/java/ghIssues/issue374/package-info.java new file mode 100644 index 00000000000..19652696a13 --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue374/package-info.java @@ -0,0 +1,5 @@ +/* + * An example to reproduce problem at https://github.com/spotbugs/spotbugs/issues/374 + */ +@javax.annotation.ParametersAreNonnullByDefault +package ghIssues.issue374; \ No newline at end of file