diff --git a/build.gradle b/build.gradle index 52325a9..a547c48 100644 --- a/build.gradle +++ b/build.gradle @@ -1,22 +1,28 @@ plugins { - id "de.marcphilipp.nexus-publish" version "0.4.0" - id "io.codearte.nexus-staging" version "0.21.1" + //id "checkstyle" + //id "pmd" + id 'maven-publish' + id 'signing' + id "io.github.gradle-nexus.publish-plugin" version "1.3.0" + //id "com.github.spotbugs" version "5.0.14" + id "org.cyclonedx.bom" version "1.7.2" + //id "org.xbib.gradle.plugin.asciidoctor" version "3.0.0" } wrapper { - gradleVersion = "${project.property('gradle.wrapper.version')}" + gradleVersion = libs.versions.gradle.get() distributionType = Wrapper.DistributionType.ALL } ext { - user = 'jprante' + user = 'joerg' name = 'guice' description = 'Guice implementation with named modules for Java 11+' inceptionYear = '2012' - url = 'https://github.com/' + user + '/' + name - scmUrl = 'https://github.com/' + user + '/' + name - scmConnection = 'scm:git:git://github.com/' + user + '/' + name + '.git' - scmDeveloperConnection = 'scm:git:ssh://git@github.com:' + user + '/' + name + '.git' + url = 'https://xbib.org/' + user + '/' + name + scmUrl = 'https://xbib.org/' + user + '/' + name + scmConnection = 'scm:git:git://xbib.org/' + user + '/' + name + '.git' + scmDeveloperConnection = 'scm:git:ssh://forgejo@xbib.org:' + user + '/' + name + '.git' issueManagementSystem = 'Github' issueManagementUrl = ext.scmUrl + '/issues' licenseName = 'The Apache License, Version 2.0' @@ -29,13 +35,17 @@ apply from: rootProject.file('gradle/ide/idea.gradle') apply from: rootProject.file('gradle/repositories/maven.gradle') apply from: rootProject.file('gradle/compile/java.gradle') apply from: rootProject.file('gradle/test/junit5.gradle') -apply from: rootProject.file('gradle/publishing/publication.gradle') -apply from: rootProject.file('gradle/publishing/sonatype.gradle') +apply from: rootProject.file('gradle/quality/cyclonedx.gradle') +//apply from: rootProject.file('gradle/quality/spotbugs.gradle') +//apply from: rootProject.file('gradle/quality/checkstyle.gradle') +//apply from: rootProject.file('gradle/quality/pmd.gradle') +apply from: rootProject.file('gradle/publish/sonatype.gradle') +apply from: rootProject.file('gradle/publish/forgejo.gradle') dependencies { - api "org.xbib:javax-inject:${project.property('javax-inject.version')}" - api "org.xbib:guava:${project.property('guava.version')}" - testImplementation "junit:junit:${project.property('junit4.version')}" + api libs.javax.inject + api libs.guava + testImplementation libs.junit4 // Helper for com.google.common.testing.GcFinalization, com.google.common.testing.EqualsTester - testImplementation "com.google.guava:guava-testlib:30.1-jre" + testImplementation libs.guava.testlib } diff --git a/gradle.properties b/gradle.properties index 41c09b0..6042065 100644 --- a/gradle.properties +++ b/gradle.properties @@ -3,10 +3,3 @@ name = guice version = 5.0.1.0 org.gradle.warning.mode = ALL -gradle.wrapper.version = 6.6.1 -javax-inject.version = 1 -guava.version = 30.1 -# test -junit.version = 5.7.2 -junit4.version = 4.13.2 -log4j.version = 2.14.1 diff --git a/gradle/compile/java.gradle b/gradle/compile/java.gradle index 54f1f87..5e99d19 100644 --- a/gradle/compile/java.gradle +++ b/gradle/compile/java.gradle @@ -3,16 +3,18 @@ apply plugin: 'java-library' java { modularity.inferModulePath.set(true) + withSourcesJar() + withJavadocJar() } compileJava { - sourceCompatibility = JavaVersion.VERSION_11 - targetCompatibility = JavaVersion.VERSION_11 + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 } compileTestJava { - sourceCompatibility = JavaVersion.VERSION_11 - targetCompatibility = JavaVersion.VERSION_11 + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 } jar { @@ -21,20 +23,6 @@ jar { } } -task sourcesJar(type: Jar, dependsOn: classes) { - classifier 'sources' - from sourceSets.main.allSource -} - -task javadocJar(type: Jar, dependsOn: javadoc) { - classifier 'javadoc' - from javadoc.destinationDir -} - -artifacts { - archives sourcesJar, javadocJar -} - tasks.withType(JavaCompile) { options.encoding('UTF-8') options.compilerArgs << '-Xlint:all' diff --git a/gradle/publish/forgejo.gradle b/gradle/publish/forgejo.gradle new file mode 100644 index 0000000..b99b2fb --- /dev/null +++ b/gradle/publish/forgejo.gradle @@ -0,0 +1,16 @@ +if (project.hasProperty('forgeJoToken')) { + publishing { + repositories { + maven { + url 'https://xbib.org/api/packages/joerg/maven' + credentials(HttpHeaderCredentials) { + name = "Authorization" + value = "token ${project.property('forgeJoToken')}" + } + authentication { + header(HttpHeaderAuthentication) + } + } + } + } +} diff --git a/gradle/publish/ivy.gradle b/gradle/publish/ivy.gradle new file mode 100644 index 0000000..71aa155 --- /dev/null +++ b/gradle/publish/ivy.gradle @@ -0,0 +1,27 @@ +apply plugin: 'ivy-publish' + +publishing { + repositories { + ivy { + url = "https://xbib.org/repo" + } + } + publications { + ivy(IvyPublication) { + from components.java + descriptor { + license { + name = 'The Apache License, Version 2.0' + url = 'http://www.apache.org/licenses/LICENSE-2.0.txt' + } + author { + name = 'Jörg Prante' + url = 'https://xbib.org/joerg' + } + descriptor.description { + text = rootProject.ext.description + } + } + } + } +} \ No newline at end of file diff --git a/gradle/publishing/publication.gradle b/gradle/publish/maven.gradle similarity index 70% rename from gradle/publishing/publication.gradle rename to gradle/publish/maven.gradle index 2e2b2c0..867e23a 100644 --- a/gradle/publishing/publication.gradle +++ b/gradle/publish/maven.gradle @@ -1,13 +1,10 @@ -apply plugin: "de.marcphilipp.nexus-publish" - publishing { publications { - mavenJava(MavenPublication) { + "${project.name}"(MavenPublication) { from components.java - artifact sourcesJar - artifact javadocJar pom { + artifactId = project.name name = project.name description = rootProject.ext.description url = rootProject.ext.url @@ -19,10 +16,10 @@ publishing { } developers { developer { - id = 'jprante' + id = 'joerg' name = 'Jörg Prante' email = 'joergprante@gmail.com' - url = 'https://github.com/jprante' + url = 'https://xbib.org/joerg' } } scm { @@ -49,18 +46,6 @@ publishing { if (project.hasProperty("signing.keyId")) { apply plugin: 'signing' signing { - sign publishing.publications.mavenJava - } -} - -if (project.hasProperty("ossrhUsername")) { - nexusPublishing { - repositories { - sonatype { - username = project.property('ossrhUsername') - password = project.property('ossrhPassword') - packageGroup = "org.xbib" - } - } + sign publishing.publications."${project.name}" } } diff --git a/gradle/publish/sonatype.gradle b/gradle/publish/sonatype.gradle new file mode 100644 index 0000000..5d739de --- /dev/null +++ b/gradle/publish/sonatype.gradle @@ -0,0 +1,11 @@ +if (project.hasProperty('ossrhUsername') && project.hasProperty('ossrhPassword')) { + nexusPublishing { + repositories { + sonatype { + username = project.property('ossrhUsername') + password = project.property('ossrhPassword') + packageGroup = "org.xbib" + } + } + } +} diff --git a/gradle/publishing/sonatype.gradle b/gradle/publishing/sonatype.gradle deleted file mode 100644 index e1813f3..0000000 --- a/gradle/publishing/sonatype.gradle +++ /dev/null @@ -1,11 +0,0 @@ - -if (project.hasProperty('ossrhUsername') && project.hasProperty('ossrhPassword')) { - - apply plugin: 'io.codearte.nexus-staging' - - nexusStaging { - username = project.property('ossrhUsername') - password = project.property('ossrhPassword') - packageGroup = "org.xbib" - } -} diff --git a/gradle/quality/checkstyle.gradle b/gradle/quality/checkstyle.gradle new file mode 100644 index 0000000..85b8bd8 --- /dev/null +++ b/gradle/quality/checkstyle.gradle @@ -0,0 +1,19 @@ + +apply plugin: 'checkstyle' + +tasks.withType(Checkstyle) { + ignoreFailures = true + reports { + xml.getRequired().set(true) + html.getRequired().set(true) + } +} + +checkstyle { + configFile = rootProject.file('gradle/quality/checkstyle.xml') + ignoreFailures = true + showViolations = true + checkstyleMain { + source = sourceSets.main.allSource + } +} diff --git a/gradle/quality/checkstyle.xml b/gradle/quality/checkstyle.xml new file mode 100644 index 0000000..66a9aae --- /dev/null +++ b/gradle/quality/checkstyle.xml @@ -0,0 +1,333 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/gradle/quality/cyclonedx.gradle b/gradle/quality/cyclonedx.gradle new file mode 100644 index 0000000..a6bf41b --- /dev/null +++ b/gradle/quality/cyclonedx.gradle @@ -0,0 +1,11 @@ +cyclonedxBom { + includeConfigs = [ 'runtimeClasspath' ] + skipConfigs = [ 'compileClasspath', 'testCompileClasspath' ] + projectType = "library" + schemaVersion = "1.4" + destination = file("build/reports") + outputName = "bom" + outputFormat = "json" + includeBomSerialNumber = true + componentVersion = "2.0.0" +} diff --git a/gradle/quality/pmd.gradle b/gradle/quality/pmd.gradle new file mode 100644 index 0000000..55fcfda --- /dev/null +++ b/gradle/quality/pmd.gradle @@ -0,0 +1,17 @@ + +apply plugin: 'pmd' + +tasks.withType(Pmd) { + ignoreFailures = true + reports { + xml.getRequired().set(true) + html.getRequired().set(true) + } +} + +pmd { + ignoreFailures = true + consoleOutput = false + toolVersion = "6.51.0" + ruleSetFiles = rootProject.files('gradle/quality/pmd/category/java/bestpractices.xml') +} diff --git a/gradle/quality/pmd/category/java/bestpractices.xml b/gradle/quality/pmd/category/java/bestpractices.xml new file mode 100644 index 0000000..6bf15a0 --- /dev/null +++ b/gradle/quality/pmd/category/java/bestpractices.xml @@ -0,0 +1,1650 @@ + + + + + + Rules which enforce generally accepted best practices. + + + + + The abstract class does not contain any abstract methods. An abstract class suggests + an incomplete implementation, which is to be completed by subclasses implementing the + abstract methods. If the class is intended to be used as a base class only (not to be instantiated + directly) a protected constructor can be provided prevent direct instantiation. + + 3 + + + + + + + + + + + + + + + Instantiation by way of private constructors from outside of the constructor's class often causes the + generation of an accessor. A factory method, or non-privatization of the constructor can eliminate this + situation. The generated class file is actually an interface. It gives the accessing class the ability + to invoke a new hidden package scope constructor that takes the interface as a supplementary parameter. + This turns a private constructor effectively into one with package scope, and is challenging to discern. + + 3 + + + + + + + + When accessing a private field / method from another class, the Java compiler will generate a accessor methods + with package-private visibility. This adds overhead, and to the dex method count on Android. This situation can + be avoided by changing the visibility of the field / method from private to package-private. + + 3 + + + + + + + + Constructors and methods receiving arrays should clone objects and store the copy. + This prevents future changes from the user from affecting the original array. + + 3 + + + + + + + + Avoid printStackTrace(); use a logger call instead. + + 3 + + + + + + + + + + + + + + + Reassigning loop variables can lead to hard-to-find bugs. Prevent or limit how these variables can be changed. + + In foreach-loops, configured by the `foreachReassign` property: + - `deny`: Report any reassignment of the loop variable in the loop body. _This is the default._ + - `allow`: Don't check the loop variable. + - `firstOnly`: Report any reassignments of the loop variable, except as the first statement in the loop body. + _This is useful if some kind of normalization or clean-up of the value before using is permitted, but any other change of the variable is not._ + + In for-loops, configured by the `forReassign` property: + - `deny`: Report any reassignment of the control variable in the loop body. _This is the default._ + - `allow`: Don't check the control variable. + - `skip`: Report any reassignments of the control variable, except conditional increments/decrements (`++`, `--`, `+=`, `-=`). + _This prevents accidental reassignments or unconditional increments of the control variable._ + + 3 + + + + + + + + Reassigning values to incoming parameters is not recommended. Use temporary local variables instead. + + 2 + + + + + + + + StringBuffers/StringBuilders can grow considerably, and so may become a source of memory leaks + if held within objects with long lifetimes. + + 3 + + + + + + + + + + + + + + + Application with hard-coded IP addresses can become impossible to deploy in some cases. + Externalizing IP adresses is preferable. + + 3 + + + + + + + + Always check the return values of navigation methods (next, previous, first, last) of a ResultSet. + If the value return is 'false', it should be handled properly. + + 3 + + + + + + + + Avoid constants in interfaces. Interfaces should define types, constants are implementation details + better placed in classes or enums. See Effective Java, item 19. + + 3 + + + + + + + + + + + + + + + + By convention, the default label should be the last label in a switch statement. + + 3 + + + + + + + + + + + + + + + Reports loops that can be safely replaced with the foreach syntax. The rule considers loops over + lists, arrays and iterators. A loop is safe to replace if it only uses the index variable to + access an element of the list or array, only has one update statement, and loops through *every* + element of the list or array left to right. + + 3 + + l) { + for (int i = 0; i < l.size(); i++) { // pre Java 1.5 + System.out.println(l.get(i)); + } + + for (String s : l) { // post Java 1.5 + System.out.println(s); + } + } +} +]]> + + + + + + Having a lot of control variables in a 'for' loop makes it harder to see what range of values + the loop iterates over. By default this rule allows a regular 'for' loop with only one variable. + + 3 + + + + //ForInit/LocalVariableDeclaration[count(VariableDeclarator) > $maximumVariables] + + + + + + + + + + Whenever using a log level, one should check if the loglevel is actually enabled, or + otherwise skip the associate String creation and manipulation. + + 2 + + + + + + + + In JUnit 3, test suites are indicated by the suite() method. In JUnit 4, suites are indicated + through the @RunWith(Suite.class) annotation. + + 3 + + + + + + + + + + + + + + + In JUnit 3, the tearDown method was used to clean up all data entities required in running tests. + JUnit 4 skips the tearDown method and executes all methods annotated with @After after running each test. + JUnit 5 introduced @AfterEach and @AfterAll annotations to execute methods after each test or after all tests in the class, respectively. + + 3 + + + + + + + + + + + + + + + In JUnit 3, the setUp method was used to set up all data entities required in running tests. + JUnit 4 skips the setUp method and executes all methods annotated with @Before before all tests. + JUnit 5 introduced @BeforeEach and @BeforeAll annotations to execute methods before each test or before all tests in the class, respectively. + + 3 + + + + + + + + + + + + + + + In JUnit 3, the framework executed all methods which started with the word test as a unit test. + In JUnit 4, only methods annotated with the @Test annotation are executed. + In JUnit 5, one of the following annotations should be used for tests: @Test, @RepeatedTest, @TestFactory, @TestTemplate or @ParameterizedTest. + + 3 + + + + + + + + + + + + + + + + + JUnit assertions should include an informative message - i.e., use the three-argument version of + assertEquals(), not the two-argument version. + + 3 + + + + + + + + Unit tests should not contain too many asserts. Many asserts are indicative of a complex test, for which + it is harder to verify correctness. Consider breaking the test scenario into multiple, shorter test scenarios. + Customize the maximum number of assertions used by this Rule to suit your needs. + + This rule checks for JUnit4, JUnit5 and TestNG Tests, as well as methods starting with "test". + + 3 + + + + + $maximumAsserts] +]]> + + + + + + + + + + + JUnit tests should include at least one assertion. This makes the tests more robust, and using assert + with messages provide the developer a clearer idea of what the test does. + + 3 + + + + + + + + In JUnit4, use the @Test(expected) annotation to denote tests that should throw exceptions. + + 3 + + + + + + + + The use of implementation types (i.e., HashSet) as object references limits your ability to use alternate + implementations in the future as requirements change. Whenever available, referencing objects + by their interface types (i.e, Set) provides much more flexibility. + + 3 + + list = new ArrayList<>(); + + public HashSet getFoo() { + return new HashSet(); + } + + // preferred approach + private List list = new ArrayList<>(); + + public Set getFoo() { + return new HashSet(); + } +} +]]> + + + + + + Exposing internal arrays to the caller violates object encapsulation since elements can be + removed or replaced outside of the object that owns it. It is safer to return a copy of the array. + + 3 + + + + + + + + + Annotating overridden methods with @Override ensures at compile time that + the method really overrides one, which helps refactoring and clarifies intent. + + 3 + + + + + + + + Java allows the use of several variables declaration of the same type on one line. However, it + can lead to quite messy code. This rule looks for several declarations on the same line. + + 4 + + + + 1] + [$strictMode or count(distinct-values(VariableDeclarator/@BeginLine)) != count(VariableDeclarator)] +| +//FieldDeclaration + [count(VariableDeclarator) > 1] + [$strictMode or count(distinct-values(VariableDeclarator/@BeginLine)) != count(VariableDeclarator)] +]]> + + + + + + + + + + + + + Position literals first in comparisons, if the second argument is null then NullPointerExceptions + can be avoided, they will just return false. + + 3 + + + + + + + + + + + + + + + Position literals first in comparisons, if the second argument is null then NullPointerExceptions + can be avoided, they will just return false. + + 3 + + + + + + + + + + + + + + + Throwing a new exception from a catch block without passing the original exception into the + new exception will cause the original stack trace to be lost making it difficult to debug + effectively. + + 3 + + + + + + + + Consider replacing Enumeration usages with the newer java.util.Iterator + + 3 + + + + + + + + + + + + + + + Consider replacing Hashtable usage with the newer java.util.Map if thread safety is not required. + + 3 + + + //Type/ReferenceType/ClassOrInterfaceType[@Image='Hashtable'] + + + + + + + + + + Consider replacing Vector usages with the newer java.util.ArrayList if expensive thread-safe operations are not required. + + 3 + + + //Type/ReferenceType/ClassOrInterfaceType[@Image='Vector'] + + + + + + + + + + All switch statements should include a default option to catch any unspecified values. + + 3 + + + + + + + + + + + + + + References to System.(out|err).print are usually intended for debugging purposes and can remain in + the codebase even in production code. By using a logger one can enable/disable this behaviour at + will (and by priority) and avoid clogging the Standard out log. + + 2 + + + + + + + + + + + + + + + Avoid passing parameters to methods or constructors without actually referencing them in the method body. + + 3 + + + + + + + + Avoid unused import statements to prevent unwanted dependencies. + This rule will also find unused on demand imports, i.e. import com.foo.*. + + 4 + + + + + + + + Detects when a local variable is declared and/or assigned, but not used. + + 3 + + + + + + + + Detects when a private field is declared and/or assigned a value, but not used. + + 3 + + + + + + + + Unused Private Method detects when a private method is declared but is unused. + + 3 + + + + + + + + This rule detects JUnit assertions in object equality. These assertions should be made by more specific methods, like assertEquals. + + 3 + + + + + + + + + + + + + + + This rule detects JUnit assertions in object references equality. These assertions should be made by + more specific methods, like assertNull, assertNotNull. + + 3 + + + + + + + + + + + + + + + This rule detects JUnit assertions in object references equality. These assertions should be made + by more specific methods, like assertSame, assertNotSame. + + 3 + + + + + + + + + + + + + + + When asserting a value is the same as a literal or Boxed boolean, use assertTrue/assertFalse, instead of assertEquals. + + 3 + + + + + + + + + + + + + + + The isEmpty() method on java.util.Collection is provided to determine if a collection has any elements. + Comparing the value of size() to 0 does not convey intent as well as the isEmpty() method. + + 3 + + + + + + + + Java 7 introduced the try-with-resources statement. This statement ensures that each resource is closed at the end + of the statement. It avoids the need of explicitly closing the resources in a finally block. Additionally exceptions + are better handled: If an exception occurred both in the `try` block and `finally` block, then the exception from + the try block was suppressed. With the `try`-with-resources statement, the exception thrown from the try-block is + preserved. + + 3 + + + + + + + + + + + + + + + + + Java 5 introduced the varargs parameter declaration for methods and constructors. This syntactic + sugar provides flexibility for users of these methods and constructors, allowing them to avoid + having to deal with the creation of an array. + + 4 + + + + + + + + + + + + + + diff --git a/gradle/quality/pmd/category/java/categories.properties b/gradle/quality/pmd/category/java/categories.properties new file mode 100644 index 0000000..8ef5eac --- /dev/null +++ b/gradle/quality/pmd/category/java/categories.properties @@ -0,0 +1,10 @@ + +rulesets.filenames=\ + category/java/bestpractices.xml,\ + category/java/codestyle.xml,\ + category/java/design.xml,\ + category/java/documentation.xml,\ + category/java/errorprone.xml,\ + category/java/multithreading.xml,\ + category/java/performance.xml,\ + category/java/security.xml diff --git a/gradle/quality/pmd/category/java/codestyle.xml b/gradle/quality/pmd/category/java/codestyle.xml new file mode 100644 index 0000000..186ea4b --- /dev/null +++ b/gradle/quality/pmd/category/java/codestyle.xml @@ -0,0 +1,2176 @@ + + + + + + Rules which enforce a specific coding style. + + + + + Abstract classes should be named 'AbstractXXX'. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by {% rule java/codestyle/ClassNamingConventions %}. + + 3 + + + + + + + + + + + + + + + + + + 3 + + + + + + + + Avoid using dollar signs in variable/method/class/interface names. + + 3 + + + + + + + Avoid using final local variables, turn them into fields. + 3 + + + + + + + + + + + + + + + Prefixing parameters by 'in' or 'out' pollutes the name of the parameters and reduces code readability. + To indicate whether or not a parameter will be modify in a method, its better to document method + behavior with Javadoc. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the more general rule {% rule java/codestyle/FormalParameterNamingConventions %}. + + 4 + + + + + + + + + + + + + + + + + + Do not use protected fields in final classes since they cannot be subclassed. + Clarify your intent by using private or package access modifiers instead. + + 3 + + + + + + + + + + + + + + + Do not use protected methods in most final classes since they cannot be subclassed. This should + only be allowed in final classes that extend other classes with protected methods (whose + visibility cannot be reduced). Clarify your intent by using private or package access modifiers instead. + + 3 + + + + + + + + + + + + + + + Unnecessary reliance on Java Native Interface (JNI) calls directly reduces application portability + and increases the maintenance burden. + + 2 + + + //Name[starts-with(@Image,'System.loadLibrary')] + + + + + + + + + + Methods that return boolean results should be named as predicate statements to denote this. + I.e, 'isReady()', 'hasValues()', 'canCommit()', 'willFail()', etc. Avoid the use of the 'get' + prefix for these methods. + + 4 + + + + + + + + + + + + + + + + It is a good practice to call super() in a constructor. If super() is not called but + another constructor (such as an overloaded constructor) is called, this rule will not report it. + + 3 + + + + 0 ] +/ClassOrInterfaceBody + /ClassOrInterfaceBodyDeclaration + /ConstructorDeclaration[ count (.//ExplicitConstructorInvocation)=0 ] +]]> + + + + + + + + + + + Configurable naming conventions for type declarations. This rule reports + type declarations which do not match the regex that applies to their + specific kind (e.g. enum or interface). Each regex can be configured through + properties. + + By default this rule uses the standard Java naming convention (Pascal case), + and reports utility class names not ending with 'Util'. + + 1 + + + + + + + + To avoid mistakes if we want that a Method, Constructor, Field or Nested class have a default access modifier + we must add a comment at the beginning of it's declaration. + By default the comment must be /* default */ or /* package */, if you want another, you have to provide a regular expression. + This rule ignores by default all cases that have a @VisibleForTesting annotation. Use the + property "ignoredAnnotations" to customize the recognized annotations. + + 3 + + + + + + + + Avoid negation within an "if" expression with an "else" clause. For example, rephrase: + `if (x != y) diff(); else same();` as: `if (x == y) same(); else diff();`. + + Most "if (x != y)" cases without an "else" are often return cases, so consistent use of this + rule makes the code easier to read. Also, this resolves trivial ordering problems, such + as "does the error case go first?" or "does the common case go first?". + + 3 + + + + + + + + Enforce a policy for braces on control statements. It is recommended to use braces on 'if ... else' + statements and loop statements, even if they are optional. This usually makes the code clearer, and + helps prepare the future when you need to add another statement. That said, this rule lets you control + which statements are required to have braces via properties. + + From 6.2.0 on, this rule supersedes WhileLoopMustUseBraces, ForLoopMustUseBraces, IfStmtMustUseBraces, + and IfElseStmtMustUseBraces. + + 3 + + + + + + + + + + + + + 1 + or (some $stmt (: in only the block statements until the next label :) + in following-sibling::BlockStatement except following-sibling::SwitchLabel[1]/following-sibling::BlockStatement + satisfies not($stmt/Statement/Block))] + ]]> + + + + + + + + + + Use explicit scoping instead of accidental usage of default package private level. + The rule allows methods and fields annotated with Guava's @VisibleForTesting. + + 3 + + + + + + + + + + + + Avoid importing anything from the package 'java.lang'. These classes are automatically imported (JLS 7.5.3). + + 4 + + + + + + + + Duplicate or overlapping import statements should be avoided. + + 4 + + + + + + + + Empty or auto-generated methods in an abstract class should be tagged as abstract. This helps to remove their inapproprate + usage by developers who should be implementing their own versions in the concrete subclasses. + + 1 + + + + + + + + + + + + + + No need to explicitly extend Object. + 4 + + + + + + + + + + + + + + + Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes. + + 3 + + + + + + + + + Configurable naming conventions for field declarations. This rule reports variable declarations + which do not match the regex that applies to their specific kind ---e.g. constants (static final), + enum constant, final field. Each regex can be configured through properties. + + By default this rule uses the standard Java naming convention (Camel case), and uses the ALL_UPPER + convention for constants and enum constants. + + 1 + + + + + + + + Some for loops can be simplified to while loops, this makes them more concise. + + 3 + + + + + + + + + + + + + + + Avoid using 'for' statements without using curly braces. If the code formatting or + indentation is lost then it becomes difficult to separate the code being controlled + from the rest. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/codestyle/ControlStatementBraces %}. + + 3 + + + //ForStatement[not(Statement/Block)] + + + + + + + + + + Configurable naming conventions for formal parameters of methods and lambdas. + This rule reports formal parameters which do not match the regex that applies to their + specific kind (e.g. lambda parameter, or final formal parameter). Each regex can be + configured through properties. + + By default this rule uses the standard Java naming convention (Camel case). + + 1 + + lambda1 = s_str -> { }; + + // lambda parameters with an explicit type can be configured separately + Consumer lambda1 = (String str) -> { }; + + } + + } + ]]> + + + + + + Names for references to generic values should be limited to a single uppercase letter. + + 4 + + + + 1 + or + string:upper-case(@Image) != @Image +] +]]> + + + + + extends BaseDao { + // This is ok... +} + +public interface GenericDao { + // Also this +} + +public interface GenericDao { + // 'e' should be an 'E' +} + +public interface GenericDao { + // 'EF' is not ok. +} +]]> + + + + + + + Identical `catch` branches use up vertical space and increase the complexity of code without + adding functionality. It's better style to collapse identical branches into a single multi-catch + branch. + + 3 + + + + + + + + Avoid using if..else statements without using surrounding braces. If the code formatting + or indentation is lost then it becomes difficult to separate the code being controlled + from the rest. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/codestyle/ControlStatementBraces %}. + + 3 + + + + + + + + + + + + + + + Avoid using if statements without using braces to surround the code block. If the code + formatting or indentation is lost then it becomes difficult to separate the code being + controlled from the rest. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/codestyle/ControlStatementBraces %}. + + 3 + + + + + + + + + + + + + + + This rule finds Linguistic Naming Antipatterns. It checks for fields, that are named, as if they should + be boolean but have a different type. It also checks for methods, that according to their name, should + return a boolean, but don't. Further, it checks, that getters return something and setters won't. + Finally, it checks that methods, that start with "to" - so called transform methods - actually return + something, since according to their name, they should convert or transform one object into another. + There is additionally an option, to check for methods that contain "To" in their name - which are + also transform methods. However, this is disabled by default, since this detection is prone to + false positives. + + For more information, see [Linguistic Antipatterns - What They Are and How + Developers Perceive Them](https://doi.org/10.1007/s10664-014-9350-8). + + 3 + + + + + + + + The Local Home interface of a Session EJB should be suffixed by 'LocalHome'. + + 4 + + + + + + + + + + + + + + + The Local Interface of a Session EJB should be suffixed by 'Local'. + + 4 + + + + + + + + + + + + + + + A local variable assigned only once can be declared final. + + 3 + + + + + + + + Configurable naming conventions for local variable declarations and other locally-scoped + variables. This rule reports variable declarations which do not match the regex that applies to their + specific kind (e.g. final variable, or catch-clause parameter). Each regex can be configured through + properties. + + By default this rule uses the standard Java naming convention (Camel case). + + 1 + + + + + + + + Fields, formal arguments, or local variable names that are too long can make the code difficult to follow. + + 3 + + + + + $minimum] +]]> + + + + + + + + + + + The EJB Specification states that any MessageDrivenBean or SessionBean should be suffixed by 'Bean'. + + 4 + + + + + + + + + + + + + + + A method argument that is never re-assigned within the method can be declared final. + + 3 + + + + + + + + Configurable naming conventions for method declarations. This rule reports + method declarations which do not match the regex that applies to their + specific kind (e.g. JUnit test or native method). Each regex can be + configured through properties. + + By default this rule uses the standard Java naming convention (Camel case). + + 1 + + + + + + + + Detects when a non-field has a name starting with 'm_'. This usually denotes a field and could be confusing. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the more general rule + {% rule java/codestyle/LocalVariableNamingConventions %}. + + 3 + + + + + + + + + + + + + + + Detects when a class or interface does not have a package definition. + + 3 + + + //ClassOrInterfaceDeclaration[count(preceding::PackageDeclaration) = 0] + + + + + + + + + + Since Java 1.7, numeric literals can use underscores to separate digits. This rule enforces that + numeric literals above a certain length use these underscores to increase readability. + + The rule only supports decimal (base 10) literals for now. The acceptable length under which literals + are not required to have underscores is configurable via a property. Even under that length, underscores + that are misplaced (not making groups of 3 digits) are reported. + + 3 + + + + + + + + + + + + + + + + + A method should have only one exit point, and that should be the last statement in the method. + + 3 + + 0) { + return "hey"; // first exit + } + return "hi"; // second exit + } +} +]]> + + + + + + Detects when a package definition contains uppercase characters. + + 3 + + + //PackageDeclaration/Name[lower-case(@Image)!=@Image] + + + + + + + + + + Checks for variables that are defined before they might be used. A reference is deemed to be premature if it is created right before a block of code that doesn't use it that also has the ability to return or throw an exception. + + 3 + + + + + + + + Remote Interface of a Session EJB should not have a suffix. + + 4 + + + + + + + + + + + + + + + A Remote Home interface type of a Session EJB should be suffixed by 'Home'. + + 4 + + + + + + + + + + + + + + + Short Classnames with fewer than e.g. five characters are not recommended. + + 4 + + + + + + + + + + + + + + + + Method names that are very short are not helpful to the reader. + + 3 + + + + + + + + + + + + + + + + Fields, local variables, or parameter names that are very short are not helpful to the reader. + + 3 + + + + + + + + + + + + + + + + + Field names using all uppercase characters - Sun's Java naming conventions indicating constants - should + be declared as final. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the more general rule {% rule java/codestyle/FieldNamingConventions %}. + + 3 + + + + + + + + + + + + + + + If you overuse the static import feature, it can make your program unreadable and + unmaintainable, polluting its namespace with all the static members you import. + Readers of your code (including you, a few months after you wrote it) will not know + which class a static member comes from (Sun 1.5 Language Guide). + + 3 + + + + + $maximumStaticImports] +]]> + + + + + + + + + + + + Avoid the use of value in annotations when it's the only element. + + 3 + + + + + + + + + This rule detects when a constructor is not necessary; i.e., when there is only one constructor and the + constructor is identical to the default constructor. The default constructor should has same access + modifier as the declaring class. In an enum type, the default constructor is implicitly private. + + 3 + + + + + + + + Import statements allow the use of non-fully qualified names. The use of a fully qualified name + which is covered by an import statement is redundant. Consider using the non-fully qualified name. + + 4 + + + + + + + + Avoid the creation of unnecessary local variables + + 3 + + + + + + + + Fields in interfaces and annotations are automatically `public static final`, and methods are `public abstract`. + Classes, interfaces or annotations nested in an interface or annotation are automatically `public static` + (all nested interfaces and annotations are automatically static). + Nested enums are automatically `static`. + For historical reasons, modifiers which are implied by the context are accepted by the compiler, but are superfluous. + + 3 + + + + + + + + Avoid the use of unnecessary return statements. + + 3 + + + + + + + + Use the diamond operator to let the type be inferred automatically. With the Diamond operator it is possible + to avoid duplication of the type parameters. + Instead, the compiler is now able to infer the parameter types for constructor calls, + which makes the code also more readable. + + 3 + + + + + + + + + strings = new ArrayList(); // unnecessary duplication of type parameters +List stringsWithDiamond = new ArrayList<>(); // using the diamond operator is more concise +]]> + + + + + Useless parentheses should be removed. + 4 + + + + 1] + /PrimaryPrefix/Expression + [not(./CastExpression)] + [not(./ConditionalExpression)] + [not(./AdditiveExpression)] + [not(./AssignmentOperator)] +| +//Expression[not(parent::PrimaryPrefix)]/PrimaryExpression[count(*)=1] + /PrimaryPrefix/Expression +| +//Expression/ConditionalAndExpression/PrimaryExpression/PrimaryPrefix/Expression[ + count(*)=1 and + count(./CastExpression)=0 and + count(./EqualityExpression/MultiplicativeExpression)=0 and + count(./ConditionalExpression)=0 and + count(./ConditionalOrExpression)=0] +| +//Expression/ConditionalOrExpression/PrimaryExpression/PrimaryPrefix/Expression[ + count(*)=1 and + not(./CastExpression) and + not(./ConditionalExpression) and + not(./EqualityExpression/MultiplicativeExpression)] +| +//Expression/ConditionalExpression/PrimaryExpression/PrimaryPrefix/Expression[ + count(*)=1 and + not(./CastExpression) and + not(./EqualityExpression)] +| +//Expression/AdditiveExpression[not(./PrimaryExpression/PrimaryPrefix/Literal[@StringLiteral='true'])] + /PrimaryExpression[1]/PrimaryPrefix/Expression[ + count(*)=1 and + not(./CastExpression) and + not(./AdditiveExpression[@Image = '-']) and + not(./ShiftExpression) and + not(./RelationalExpression) and + not(./InstanceOfExpression) and + not(./EqualityExpression) and + not(./AndExpression) and + not(./ExclusiveOrExpression) and + not(./InclusiveOrExpression) and + not(./ConditionalAndExpression) and + not(./ConditionalOrExpression) and + not(./ConditionalExpression)] +| +//Expression/EqualityExpression/PrimaryExpression/PrimaryPrefix/Expression[ + count(*)=1 and + not(./CastExpression) and + not(./AndExpression) and + not(./InclusiveOrExpression) and + not(./ExclusiveOrExpression) and + not(./ConditionalExpression) and + not(./ConditionalAndExpression) and + not(./ConditionalOrExpression) and + not(./EqualityExpression)] +]]> + + + + + + + + + + + Reports qualified this usages in the same class. + + 3 + + + + + + + + + + + + + + + A variable naming conventions rule - customize this to your liking. Currently, it + checks for final variables that should be fully capitalized and non-final variables + that should not include underscores. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the more general rules {% rule java/codestyle/FieldNamingConventions %}, + {% rule java/codestyle/FormalParameterNamingConventions %}, and + {% rule java/codestyle/LocalVariableNamingConventions %}. + + 1 + + + + + + + + Avoid using 'while' statements without using braces to surround the code block. If the code + formatting or indentation is lost then it becomes difficult to separate the code being + controlled from the rest. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/codestyle/ControlStatementBraces %}. + + 3 + + + //WhileStatement[not(Statement/Block)] + + + + + + + + diff --git a/gradle/quality/pmd/category/java/design.xml b/gradle/quality/pmd/category/java/design.xml new file mode 100644 index 0000000..ded3d80 --- /dev/null +++ b/gradle/quality/pmd/category/java/design.xml @@ -0,0 +1,1657 @@ + + + + + + Rules that help you discover design issues. + + + + + If an abstract class does not provides any methods, it may be acting as a simple data container + that is not meant to be instantiated. In this case, it is probably better to use a private or + protected constructor in order to prevent instantiation than make the class misleadingly abstract. + + 1 + + + + + + + + + + + + + + + Avoid catching generic exceptions such as NullPointerException, RuntimeException, Exception in try-catch block + + 3 + + + + + + + + + + + + + + + Avoid creating deeply nested if-then statements since they are harder to read and error-prone to maintain. + + 3 + + y) { + if (y>z) { + if (z==x) { + // !! too deep + } + } + } + } +} +]]> + + + + + + Catch blocks that merely rethrow a caught exception only add to code size and runtime complexity. + + 3 + + + + + + + + + + + + + + + Catch blocks that merely rethrow a caught exception wrapped inside a new instance of the same type only add to + code size and runtime complexity. + + 3 + + + + + + + + + + + + + + + *Effective Java, 3rd Edition, Item 72: Favor the use of standard exceptions* +> +>Arguably, every erroneous method invocation boils down to an illegal argument or state, +but other exceptions are standardly used for certain kinds of illegal arguments and states. +If a caller passes null in some parameter for which null values are prohibited, convention dictates that +NullPointerException be thrown rather than IllegalArgumentException. + +To implement that, you are encouraged to use `java.util.Objects.requireNonNull()` +(introduced in Java 1.7). This method is designed primarily for doing parameter +validation in methods and constructors with multiple parameters. + +Your parameter validation could thus look like the following: +``` +public class Foo { + private String exampleValue; + + void setExampleValue(String exampleValue) { + // check, throw and assignment in a single standard call + this.exampleValue = Objects.requireNonNull(exampleValue, "exampleValue must not be null!"); + } + } +``` +]]> + + 1 + + + + + + + + + + + + + + + Avoid throwing certain exception types. Rather than throw a raw RuntimeException, Throwable, + Exception, or Error, use a subclassed exception or error instead. + + 1 + + + + + + + + + + + + + + + A class with only private constructors should be final, unless the private constructor + is invoked by a inner class. + + 1 + + + + = 1 ] +[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[(@Public = 'true') or (@Protected = 'true') or (@PackagePrivate = 'true')]) = 0 ] +[not(.//ClassOrInterfaceDeclaration)] +]]> + + + + + + + + + + + Sometimes two consecutive 'if' statements can be consolidated by separating their conditions with a boolean short-circuit operator. + + 3 + + + + + + + + + + + + + + + This rule counts the number of unique attributes, local variables, and return types within an object. + A number higher than the specified threshold can indicate a high degree of coupling. + + 3 + + + + + + + = 10. +Additionnally, classes with many methods of moderate complexity get reported as well once the total of their +methods' complexities reaches 80, even if none of the methods was directly reported. + +Reported methods should be broken down into several smaller methods. Reported classes should probably be broken down +into subcomponents.]]> + + 3 + + + + + + + + Data Classes are simple data holders, which reveal most of their state, and + without complex functionality. The lack of functionality may indicate that + their behaviour is defined elsewhere, which is a sign of poor data-behaviour + proximity. By directly exposing their internals, Data Classes break encapsulation, + and therefore reduce the system's maintainability and understandability. Moreover, + classes tend to strongly rely on their data representation, which makes for a brittle + design. + + Refactoring a Data Class should focus on restoring a good data-behaviour proximity. In + most cases, that means moving the operations defined on the data back into the class. + In some other cases it may make sense to remove entirely the class and move the data + into the former client classes. + + 3 + + + + + + + + Errors are system exceptions. Do not extend them. + + 3 + + + + + + + + + + + + + + + Using Exceptions as form of flow control is not recommended as they obscure true exceptions when debugging. + Either add the necessary validation or use an alternate control structure. + + 3 + + + + + + + + Excessive class file lengths are usually indications that the class may be burdened with excessive + responsibilities that could be provided by external classes or functions. In breaking these methods + apart the code becomes more manageable and ripe for reuse. + + 3 + + + + + + + + A high number of imports can indicate a high degree of coupling within an object. This rule + counts the number of unique imports and reports a violation if the count is above the + user-specified threshold. + + 3 + + + + + + + + When methods are excessively long this usually indicates that the method is doing more than its + name/signature might suggest. They also become challenging for others to digest since excessive + scrolling causes readers to lose focus. + Try to reduce the method length by creating helper methods and removing any copy/pasted code. + + 3 + + + + + + + + Methods with numerous parameters are a challenge to maintain, especially if most of them share the + same datatype. These situations usually denote the need for new objects to wrap the numerous parameters. + + 3 + + + + + + + + Classes with large numbers of public methods and attributes require disproportionate testing efforts + since combinational side effects grow rapidly and increase risk. Refactoring these classes into + smaller ones not only increases testability and reliability but also allows new variations to be + developed easily. + + 3 + + + + + + + + If a final field is assigned to a compile-time constant, it could be made static, thus saving overhead + in each object at runtime. + + 3 + + + + + + + + + + + + + + + The God Class rule detects the God Class design flaw using metrics. God classes do too many things, + are very big and overly complex. They should be split apart to be more object-oriented. + The rule uses the detection strategy described in "Object-Oriented Metrics in Practice". + The violations are reported against the entire class. + + See also the references: + + Michele Lanza and Radu Marinescu. Object-Oriented Metrics in Practice: + Using Software Metrics to Characterize, Evaluate, and Improve the Design + of Object-Oriented Systems. Springer, Berlin, 1 edition, October 2006. Page 80. + + 3 + + + + + Identifies private fields whose values never change once object initialization ends either in the declaration + of the field or by a constructor. This helps in converting existing classes to becoming immutable ones. + + 3 + + + + + + + + The Law of Demeter is a simple rule, that says "only talk to friends". It helps to reduce coupling between classes + or objects. + + See also the references: + + * Andrew Hunt, David Thomas, and Ward Cunningham. The Pragmatic Programmer. From Journeyman to Master. Addison-Wesley Longman, Amsterdam, October 1999.; + * K.J. Lieberherr and I.M. Holland. Assuring good style for object-oriented programs. Software, IEEE, 6(5):38–48, 1989.; + * <http://www.ccs.neu.edu/home/lieber/LoD.html> + * <http://en.wikipedia.org/wiki/Law_of_Demeter> + + 3 + + + + + + + + Use opposite operator instead of negating the whole expression with a logic complement operator. + + 3 + + + + + + + + + = + return false; + } + + return true; +} +]]> + + + + + + Avoid using classes from the configured package hierarchy outside of the package hierarchy, + except when using one of the configured allowed classes. + + 3 + + + + + + + + Complexity directly affects maintenance costs is determined by the number of decision points in a method + plus one for the method entry. The decision points include 'if', 'while', 'for', and 'case labels' calls. + Generally, numbers ranging from 1-4 denote low complexity, 5-7 denote moderate complexity, 8-10 denote + high complexity, and 11+ is very high complexity. Modified complexity treats switch statements as a single + decision point. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/design/CyclomaticComplexity %}. + + 3 + + + + + + + + This rule uses the NCSS (Non-Commenting Source Statements) algorithm to determine the number of lines + of code for a given constructor. NCSS ignores comments, and counts actual statements. Using this algorithm, + lines of code that are split are counted as one. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/design/NcssCount %}. + + 3 + + + + + + + + This rule uses the NCSS (Non-Commenting Source Statements) metric to determine the number of lines + of code in a class, method or constructor. NCSS ignores comments, blank lines, and only counts actual + statements. For more details on the calculation, see the documentation of + the [NCSS metric](/pmd_java_metrics_index.html#non-commenting-source-statements-ncss). + + 3 + + + + + + + + This rule uses the NCSS (Non-Commenting Source Statements) algorithm to determine the number of lines + of code for a given method. NCSS ignores comments, and counts actual statements. Using this algorithm, + lines of code that are split are counted as one. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/design/NcssCount %}. + + 3 + + + + + + + + This rule uses the NCSS (Non-Commenting Source Statements) algorithm to determine the number of lines + of code for a given type. NCSS ignores comments, and counts actual statements. Using this algorithm, + lines of code that are split are counted as one. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/design/NcssCount %}. + + 3 + + + + + + + + The NPath complexity of a method is the number of acyclic execution paths through that method. + While cyclomatic complexity counts the number of decision points in a method, NPath counts the number of + full paths from the beginning to the end of the block of the method. That metric grows exponentially, as + it multiplies the complexity of statements in the same block. For more details on the calculation, see the + documentation of the [NPath metric](/pmd_java_metrics_index.html#npath-complexity-npath). + + A threshold of 200 is generally considered the point where measures should be taken to reduce + complexity and increase readability. + + 3 + + + + + + + + A method/constructor shouldn't explicitly throw the generic java.lang.Exception, since it + is unclear which exceptions that can be thrown from the methods. It might be + difficult to document and understand such vague interfaces. Use either a class + derived from RuntimeException or a checked exception. + + 3 + + + + + + + + + + 3 + + + + + + + + + + + + + + + Avoid negation in an assertTrue or assertFalse test. + + For example, rephrase: + + assertTrue(!expr); + + as: + + assertFalse(expr); + + + 3 + + + + + + + + + + + + + + + Avoid unnecessary comparisons in boolean expressions, they serve no purpose and impacts readability. + + 3 + + + + + + + + + + + + + + + Avoid unnecessary if-then-else statements when returning a boolean. The result of + the conditional test can be returned instead. + + 3 + + + + + + + + No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument. + + 3 + + + + + + + + + + + + + + + Fields whose scopes are limited to just single methods do not rely on the containing + object to provide them to other methods. They may be better implemented as local variables + within those methods. + + 3 + + + + + + + + Complexity directly affects maintenance costs is determined by the number of decision points in a method + plus one for the method entry. The decision points include 'if', 'while', 'for', and 'case labels' calls. + Generally, numbers ranging from 1-4 denote low complexity, 5-7 denote moderate complexity, 8-10 denote + high complexity, and 11+ is very high complexity. + + This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced + by the rule {% rule java/design/CyclomaticComplexity %}. + + 3 + + + + + + + + A high ratio of statements to labels in a switch statement implies that the switch statement + is overloaded. Consider moving the statements into new methods or creating subclasses based + on the switch variable. + + 3 + + + + + + + + Classes that have too many fields can become unwieldy and could be redesigned to have fewer fields, + possibly through grouping related fields in new objects. For example, a class with individual + city/state/zip fields could park them within a single Address field. + + 3 + + + + + + + + A class with too many methods is probably a good suspect for refactoring, in order to reduce its + complexity and find a way to have more fine grained objects. + + 3 + + + + + + $maxmethods + ] +]]> + + + + + + + + The overriding method merely calls the same method defined in a superclass. + + 3 + + + + + + + + When you write a public method, you should be thinking in terms of an API. If your method is public, it means other class + will use it, therefore, you want (or need) to offer a comprehensive and evolutive API. If you pass a lot of information + as a simple series of Strings, you may think of using an Object to represent all those information. You'll get a simpler + API (such as doWork(Workload workload), rather than a tedious series of Strings) and more importantly, if you need at some + point to pass extra data, you'll be able to do so by simply modifying or extending Workload without any modification to + your API. + + 3 + + + + 3 +] +]]> + + + + + + + + + + + For classes that only have static methods, consider making them utility classes. + Note that this doesn't apply to abstract classes, since their subclasses may + well include non-static methods. Also, if you want this class to be a utility class, + remember to add a private constructor to prevent instantiation. + (Note, that this use was known before PMD 5.1.0 as UseSingleton). + + 3 + + + + + + diff --git a/gradle/quality/pmd/category/java/documentation.xml b/gradle/quality/pmd/category/java/documentation.xml new file mode 100644 index 0000000..34b351a --- /dev/null +++ b/gradle/quality/pmd/category/java/documentation.xml @@ -0,0 +1,144 @@ + + + + + + Rules that are related to code documentation. + + + + + A rule for the politically correct... we don't want to offend anyone. + + 3 + + + + + + + + Denotes whether comments are required (or unwanted) for specific language elements. + + 3 + + + + + + + + Determines whether the dimensions of non-header comments found are within the specified limits. + + 3 + + + + + + + + Uncommented Empty Constructor finds instances where a constructor does not + contain statements, but there is no comment. By explicitly commenting empty + constructors it is easier to distinguish between intentional (commented) + and unintentional empty constructors. + + 3 + + + + + + + + + + + + + + + + Uncommented Empty Method Body finds instances where a method body does not contain + statements, but there is no comment. By explicitly commenting empty method bodies + it is easier to distinguish between intentional (commented) and unintentional + empty methods. + + 3 + + + + + + + + + + + + + \ No newline at end of file diff --git a/gradle/quality/pmd/category/java/errorprone.xml b/gradle/quality/pmd/category/java/errorprone.xml new file mode 100644 index 0000000..5ee4e89 --- /dev/null +++ b/gradle/quality/pmd/category/java/errorprone.xml @@ -0,0 +1,3383 @@ + + + + + + Rules to detect constructs that are either broken, extremely confusing or prone to runtime errors. + + + + + Avoid assignments in operands; this can make code more complicated and harder to read. + + 3 + + + + + + + + Identifies a possible unsafe usage of a static field. + + 3 + + + + + + + + Methods such as getDeclaredConstructors(), getDeclaredConstructor(Class[]) and setAccessible(), + as the interface PrivilegedAction, allow for the runtime alteration of variable, class, or + method visibility, even if they are private. This violates the principle of encapsulation. + + 3 + + + + + + + + + + + + + + + Use of the term 'assert' will conflict with newer versions of Java since it is a reserved word. + + 2 + + + //VariableDeclaratorId[@Image='assert'] + + + + + + + + + + Using a branching statement as the last message of a loop may be a bug, and/or is confusing. + Ensure that the usage is not a bug, or consider using another approach. + + 2 + + 25) { + break; + } +} +]]> + + + + + + The method Object.finalize() is called by the garbage collector on an object when garbage collection determines + that there are no more references to the object. It should not be invoked by application logic. + + Note that Oracle has declared Object.finalize() as deprecated since JDK 9. + + 3 + + + + + + + + Code should never throw NullPointerExceptions under normal circumstances. A catch block may hide the + original error, causing other, more subtle problems later on. + + 3 + + + + + + + + + + + + + + + Catching Throwable errors is not recommended since its scope is very broad. It includes runtime issues such as + OutOfMemoryError that should be exposed and managed separately. + + 3 + + + + + + + + One might assume that the result of "new BigDecimal(0.1)" is exactly equal to 0.1, but it is actually + equal to .1000000000000000055511151231257827021181583404541015625. + This is because 0.1 cannot be represented exactly as a double (or as a binary fraction of any finite + length). Thus, the long value that is being passed in to the constructor is not exactly equal to 0.1, + appearances notwithstanding. + + The (String) constructor, on the other hand, is perfectly predictable: 'new BigDecimal("0.1")' is + exactly equal to 0.1, as one would expect. Therefore, it is generally recommended that the + (String) constructor be used in preference to this one. + + 3 + + + + + + + + + + + + + + + Code containing duplicate String literals can usually be improved by declaring the String as a constant field. + + 3 + + + + + + + + Use of the term 'enum' will conflict with newer versions of Java since it is a reserved word. + + 2 + + + //VariableDeclaratorId[@Image='enum'] + + + + + + + + + + It can be confusing to have a field name with the same name as a method. While this is permitted, + having information (field) and actions (method) is not clear naming. Developers versed in + Smalltalk often prefer this approach as the methods denote accessor methods. + + 3 + + + + + + + + It is somewhat confusing to have a field name matching the declaring class name. + This probably means that type and/or field names should be chosen more carefully. + + 3 + + + + + + + + Each caught exception type should be handled in its own catch clause. + + 3 + + + + + + + + + + + + + + + Avoid using hard-coded literals in conditional statements. By declaring them as static variables + or private members with descriptive names maintainability is enhanced. By default, the literals "-1" and "0" are ignored. + More exceptions can be defined with the property "ignoreMagicNumbers". + + 3 + + + + + + + + + + + = 0) { } // alternative approach + + if (aDouble > 0.0) {} // magic number 0.0 + if (aDouble >= Double.MIN_VALUE) {} // preferred approach +} +]]> + + + + + + Statements in a catch block that invoke accessors on the exception without using the information + only add to code size. Either remove the invocation, or use the return result. + + 2 + + + + + + + + + + + + + + + The use of multiple unary operators may be problematic, and/or confusing. + Ensure that the intended usage is not a bug, or consider simplifying the expression. + + 2 + + + + + + + + Integer literals should not start with zero since this denotes that the rest of literal will be + interpreted as an octal value. + + 3 + + + + + + + + Avoid equality comparisons with Double.NaN. Due to the implicit lack of representation + precision when comparing floating point numbers these are likely to cause logic errors. + + 3 + + + + + + + + + + + + + + + If a class is a bean, or is referenced by a bean directly or indirectly it needs to be serializable. + Member variables need to be marked as transient, static, or have accessor methods in the class. Marking + variables as transient is the safest and easiest modification. Accessor methods should follow the Java + naming conventions, i.e. for a variable named foo, getFoo() and setFoo() accessor methods should be provided. + + 3 + + + + + + + + The null check is broken since it will throw a NullPointerException itself. + It is likely that you used || instead of && or vice versa. + + 2 + + + + + + + Super should be called at the start of the method + 3 + + + + + + + + + + + + + + + Super should be called at the end of the method + + 3 + + + + + + + + + + + + + + + The skip() method may skip a smaller number of bytes than requested. Check the returned value to find out if it was the case or not. + + 3 + + + + + + + + When deriving an array of a specific class from your Collection, one should provide an array of + the same class as the parameter of the toArray() method. Doing otherwise you will will result + in a ClassCastException. + + 3 + + + + + + + + + + + + + + + The java Manual says "By convention, classes that implement this interface should override + Object.clone (which is protected) with a public method." + + 3 + + + + + + + + + + + + + + + The method clone() should only be implemented if the class implements the Cloneable interface with the exception of + a final method that only throws CloneNotSupportedException. + + The rule can also detect, if the class implements or extends a Cloneable class. + + 3 + + + + + + + + If a class implements cloneable the return type of the method clone() must be the class name. That way, the caller + of the clone method doesn't need to cast the returned clone to the correct type. + + Note: This is only possible with Java 1.5 or higher. + + 3 + + + + + + + + + + + + + + + The method clone() should throw a CloneNotSupportedException. + + 3 + + + + + + + + + + + + + + + Ensure that resources (like Connection, Statement, and ResultSet objects) are always closed after use. + + 3 + + + + + + + + Use equals() to compare object references; avoid comparing them with ==. + + 3 + + + + + + + + Calling overridable methods during construction poses a risk of invoking methods on an incompletely + constructed object and can be difficult to debug. + It may leave the sub-class unable to construct its superclass or forced to replicate the construction + process completely within itself, losing the ability to call super(). If the default constructor + contains a call to an overridable method, the subclass may be completely uninstantiable. Note that + this includes method calls throughout the control flow graph - i.e., if a constructor Foo() calls a + private method bar() that calls a public method buz(), this denotes a problem. + + 1 + + + + + + + The dataflow analysis tracks local definitions, undefinitions and references to variables on different paths on the data flow. + From those informations there can be found various problems. + + 1. UR - Anomaly: There is a reference to a variable that was not defined before. This is a bug and leads to an error. + 2. DU - Anomaly: A recently defined variable is undefined. These anomalies may appear in normal source text. + 3. DD - Anomaly: A recently defined variable is redefined. This is ominous but don't have to be a bug. + + 5 + + dd-anomaly + foo(buz); + buz = 2; +} // buz is undefined when leaving scope -> du-anomaly +]]> + + + + + + Calls to System.gc(), Runtime.getRuntime().gc(), and System.runFinalization() are not advised. Code should have the + same behavior whether the garbage collection is disabled using the option -Xdisableexplicitgc or not. + Moreover, "modern" jvms do a very good job handling garbage collections. If memory usage issues unrelated to memory + leaks develop within an application, it should be dealt with JVM options rather than within the code itself. + + 2 + + + + + + + + + + + + + + + Web applications should not call System.exit(), since only the web container or the + application server should stop the JVM. This rule also checks for the equivalent call Runtime.getRuntime().exit(). + + 3 + + + + + + + + + + + + + + + Extend Exception or RuntimeException instead of Throwable. + + 3 + + + + + + + + + + + + + + + Use Environment.getExternalStorageDirectory() instead of "/sdcard" + + 3 + + + //Literal[starts-with(@Image,'"/sdcard')] + + + + + + + + + + Throwing exceptions within a 'finally' block is confusing since they may mask other exceptions + or code defects. + Note: This is a PMD implementation of the Lint4j rule "A throw in a finally block" + + 4 + + + //FinallyStatement[descendant::ThrowStatement] + + + + + + + + + + Avoid importing anything from the 'sun.*' packages. These packages are not portable and are likely to change. + + 4 + + + + + + + + Don't use floating point for loop indices. If you must use floating point, use double + unless you're certain that float provides enough precision and you have a compelling + performance need (space or time). + + 3 + + + + + + + + + + + + + + + Empty Catch Block finds instances where an exception is caught, but nothing is done. + In most circumstances, this swallows an exception which should either be acted on + or reported. + + 3 + + + + + + + + + + + + + + + + + Empty finalize methods serve no purpose and should be removed. Note that Oracle has declared Object.finalize() as deprecated since JDK 9. + + 3 + + + + + + + + + + + + + + + Empty finally blocks serve no purpose and should be removed. + + 3 + + + + + + + + + + + + + + + Empty If Statement finds instances where a condition is checked but nothing is done about it. + + 3 + + + + + + + + + + + + + + + Empty initializers serve no purpose and should be removed. + + 3 + + + //Initializer/Block[count(*)=0] + + + + + + + + + + Empty block statements serve no purpose and should be removed. + + 3 + + + //BlockStatement/Statement/Block[count(*) = 0] + + + + + + + + + + An empty statement (or a semicolon by itself) that is not used as the sole body of a 'for' + or 'while' loop is probably a bug. It could also be a double semicolon, which has no purpose + and should be removed. + + 3 + + + + + + + + + + + + + + + Empty switch statements serve no purpose and should be removed. + + 3 + + + //SwitchStatement[count(*) = 1] + + + + + + + + + + Empty synchronized blocks serve no purpose and should be removed. + + 3 + + + //SynchronizedStatement/Block[1][count(*) = 0] + + + + + + + + + + Avoid empty try blocks - what's the point? + + 3 + + + + + + + + + + + + + + + Empty While Statement finds all instances where a while statement does nothing. + If it is a timing loop, then you should use Thread.sleep() for it; if it is + a while loop that does a lot in the exit expression, rewrite it to make it clearer. + + 3 + + + + + + + + + + + + + + + Tests for null should not use the equals() method. The '==' operator should be used instead. + + 1 + + + + + + + + + + + + + + + If the finalize() is implemented, its last action should be to call super.finalize. Note that Oracle has declared Object.finalize() as deprecated since JDK 9. + + 3 + + + + + + + + + + + + + + + + If the finalize() is implemented, it should do something besides just calling super.finalize(). Note that Oracle has declared Object.finalize() as deprecated since JDK 9. + + 3 + + + + + + + + + + + + + + + Methods named finalize() should not have parameters. It is confusing and most likely an attempt to + overload Object.finalize(). It will not be called by the VM. + + Note that Oracle has declared Object.finalize() as deprecated since JDK 9. + + 3 + + + + 0]] +]]> + + + + + + + + + + + When overriding the finalize(), the new method should be set as protected. If made public, + other classes may invoke it at inappropriate times. + + Note that Oracle has declared Object.finalize() as deprecated since JDK 9. + + 3 + + + + + + + + + + + + + + + Avoid idempotent operations - they have no effect. + + 3 + + + + + + + + There is no need to import a type that lives in the same package. + + 3 + + + + + + + + Avoid instantiating an object just to call getClass() on it; use the .class public member instead. + + 4 + + + + + + + + + + + + + + + Check for messages in slf4j loggers with non matching number of arguments and placeholders. + + 5 + + + + + + + + Avoid jumbled loop incrementers - its usually a mistake, and is confusing even if intentional. + + 3 + + + + + + + + + + + + + + Some JUnit framework methods are easy to misspell. + + 3 + + + + + + + + + + + + + + + The suite() method in a JUnit test needs to be both public and static. + + 3 + + + + + + + + + + + + + + + In most cases, the Logger reference can be declared as static and final. + + 2 + + + + + + + + + + + + + + + Non-constructor methods should not have the same name as the enclosing class. + + 3 + + + + + + + + The null check here is misplaced. If the variable is null a NullPointerException will be thrown. + Either the check is useless (the variable will never be "null") or it is incorrect. + + 3 + + + + + + + + + + + + + + + + + + Switch statements without break or return statements for each case option + may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through. + + 3 + + + + + + + + + + + + + + + Serializable classes should provide a serialVersionUID field. + The serialVersionUID field is also needed for abstract base classes. Each individual class in the inheritance + chain needs an own serialVersionUID field. See also [Should an abstract class have a serialVersionUID](https://stackoverflow.com/questions/893259/should-an-abstract-class-have-a-serialversionuid). + + 3 + + + + + + + + + + + + + + + A class that has private constructors and does not have any static methods or fields cannot be used. + + 3 + + + + + + + + + + + + + + + Normally only one logger is used in each class. + + 2 + + + + + + + + A non-case label (e.g. a named break/continue label) was present in a switch statement. + This legal, but confusing. It is easy to mix up the case labels and the non-case labels. + + 3 + + + //SwitchStatement//BlockStatement/Statement/LabeledStatement + + + + + + + + + + A non-static initializer block will be called any time a constructor is invoked (just prior to + invoking the constructor). While this is a valid language construct, it is rarely used and is + confusing. + + 3 + + + + + + + + + + + + + + + Assigning a "null" to a variable (outside of its declaration) is usually bad form. Sometimes, this type + of assignment is an indication that the programmer doesn't completely understand what is going on in the code. + + NOTE: This sort of assignment may used in some cases to dereference objects and encourage garbage collection. + + 3 + + + + + + + + Override both public boolean Object.equals(Object other), and public int Object.hashCode(), or override neither. Even if you are inheriting a hashCode() from a parent class, consider implementing hashCode and explicitly delegating to your superclass. + + 3 + + + + + + + + Object clone() should be implemented with super.clone(). + + 2 + + + + 0 +] +]]> + + + + + + + + + + + A logger should normally be defined private static final and be associated with the correct class. + Private final Log log; is also allowed for rare cases where loggers need to be passed around, + with the restriction that the logger needs to be passed into the constructor. + + 3 + + + + + + + + + + + + + + + + For any method that returns an array, it is a better to return an empty array rather than a + null reference. This removes the need for null checking all results and avoids inadvertent + NullPointerExceptions. + + 1 + + + + + + + + + + + + + + + Avoid returning from a finally block, this can discard exceptions. + + 3 + + + + //FinallyStatement//ReturnStatement except //FinallyStatement//(MethodDeclaration|LambdaExpression)//ReturnStatement + + + + + + + + + + Be sure to specify a Locale when creating SimpleDateFormat instances to ensure that locale-appropriate + formatting is used. + + 3 + + + + + + + + + + + + + + + Some classes contain overloaded getInstance. The problem with overloaded getInstance methods + is that the instance created using the overloaded method is not cached and so, + for each call and new objects will be created for every invocation. + + 2 + + + + + + + + Some classes contain overloaded getInstance. The problem with overloaded getInstance methods + is that the instance created using the overloaded method is not cached and so, + for each call and new objects will be created for every invocation. + + 2 + + + + + + + + According to the J2EE specification, an EJB should not have any static fields + with write access. However, static read-only fields are allowed. This ensures proper + behavior especially when instances are distributed by the container on several JREs. + + 3 + + + + + + + + + + + + + + + Individual character values provided as initialization arguments will be converted into integers. + This can lead to internal buffer sizes that are larger than expected. Some examples: + + ``` + new StringBuffer() // 16 + new StringBuffer(6) // 6 + new StringBuffer("hello world") // 11 + 16 = 27 + new StringBuffer('A') // chr(A) = 65 + new StringBuffer("A") // 1 + 16 = 17 + + new StringBuilder() // 16 + new StringBuilder(6) // 6 + new StringBuilder("hello world") // 11 + 16 = 27 + new StringBuilder('C') // chr(C) = 67 + new StringBuilder("A") // 1 + 16 = 17 + ``` + + 4 + + + + + + + + + + + + + + + The method name and parameter number are suspiciously close to equals(Object), which can denote an + intention to override the equals(Object) method. + + 2 + + + + + + + + + + + + + + + The method name and return type are suspiciously close to hashCode(), which may denote an intention + to override the hashCode() method. + + 3 + + + + + + + + A suspicious octal escape sequence was found inside a String literal. + The Java language specification (section 3.10.6) says an octal + escape sequence inside a literal String shall consist of a backslash + followed by: + + OctalDigit | OctalDigit OctalDigit | ZeroToThree OctalDigit OctalDigit + + Any octal escape sequence followed by non-octal digits can be confusing, + e.g. "\038" is interpreted as the octal escape sequence "\03" followed by + the literal character "8". + + 3 + + + + + + + + Test classes end with the suffix Test. Having a non-test class with that name is not a good practice, + since most people will assume it is a test case. Test classes have test methods named testXXX. + + 3 + + + + + + + + Do not use "if" statements whose conditionals are always true or always false. + + 3 + + + + + + + + + + + + + + + A JUnit test assertion with a boolean literal is unnecessary since it always will evaluate to the same thing. + Consider using flow control (in case of assertTrue(false) or similar) or simply removing + statements like assertTrue(true) and assertFalse(false). If you just want a test to halt after finding + an error, use the fail() method and provide an indication message of why it did. + + 3 + + + + + + + + + + + + + + + Using equalsIgnoreCase() is faster than using toUpperCase/toLowerCase().equals() + + 3 + + + + + + + + Avoid the use temporary objects when converting primitives to Strings. Use the static conversion methods + on the wrapper classes instead. + + 3 + + + + + + + + After checking an object reference for null, you should invoke equals() on that object rather than passing it to another object's equals() method. + + 3 + + + + + + + + + + + + + + + To make sure the full stacktrace is printed out, use the logging statement with two arguments: a String and a Throwable. + + 3 + + + + + + + + + + + + + + + Using '==' or '!=' to compare strings only works if intern version is used on both sides. + Use the equals() method instead. + + 3 + + + + + + + + + + + + + + + An operation on an Immutable object (String, BigDecimal or BigInteger) won't change the object itself + since the result of the operation is a new object. Therefore, ignoring the operation result is an error. + + 3 + + + + + + + + When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that + have unusual conventions, i.e. Turkish. + + 3 + + + + + + + + + + + + + + + In J2EE, the getClassLoader() method might not work as expected. Use + Thread.currentThread().getContextClassLoader() instead. + + 3 + + + //PrimarySuffix[@Image='getClassLoader'] + + + + + + + + diff --git a/gradle/quality/pmd/category/java/multithreading.xml b/gradle/quality/pmd/category/java/multithreading.xml new file mode 100644 index 0000000..d3e8327 --- /dev/null +++ b/gradle/quality/pmd/category/java/multithreading.xml @@ -0,0 +1,393 @@ + + + + + + Rules that flag issues when dealing with multiple threads of execution. + + + + + Method-level synchronization can cause problems when new code is added to the method. + Block-level synchronization helps to ensure that only the code that needs synchronization + gets it. + + 3 + + + //MethodDeclaration[@Synchronized='true'] + + + + + + + + + + Avoid using java.lang.ThreadGroup; although it is intended to be used in a threaded environment + it contains methods that are not thread-safe. + + 3 + + + + + + + + + + + + + + + Use of the keyword 'volatile' is generally used to fine tune a Java application, and therefore, requires + a good expertise of the Java Memory Model. Moreover, its range of action is somewhat misknown. Therefore, + the volatile keyword should not be used for maintenance purpose and portability. + + 2 + + + //FieldDeclaration[contains(@Volatile,'true')] + + + + + + + + + + The J2EE specification explicitly forbids the use of threads. + + 3 + + + //ClassOrInterfaceType[@Image = 'Thread' or @Image = 'Runnable'] + + + + + + + + + + Explicitly calling Thread.run() method will execute in the caller's thread of control. Instead, call Thread.start() for the intended behavior. + + 4 + + + + + + + + + + + + + + + Partially created objects can be returned by the Double Checked Locking pattern when used in Java. + An optimizing JRE may assign a reference to the baz variable before it calls the constructor of the object the + reference points to. + + Note: With Java 5, you can make Double checked locking work, if you declare the variable to be `volatile`. + + For more details refer to: <http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html> + or <http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html> + + 1 + + + + + + + + Non-thread safe singletons can result in bad state changes. Eliminate + static singletons if possible by instantiating the object directly. Static + singletons are usually not needed as only a single instance exists anyway. + Other possible fixes are to synchronize the entire method or to use an + [initialize-on-demand holder class](https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom). + + Refrain from using the double-checked locking pattern. The Java Memory Model doesn't + guarantee it to work unless the variable is declared as `volatile`, adding an uneeded + performance penalty. [Reference](http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html) + + See Effective Java, item 48. + + 3 + + + + + + + + SimpleDateFormat instances are not synchronized. Sun recommends using separate format instances + for each thread. If multiple threads must access a static formatter, the formatter must be + synchronized either on method or block level. + + This rule has been deprecated in favor of the rule {% rule UnsynchronizedStaticFormatter %}. + + 3 + + + + + + + + Instances of `java.text.Format` are generally not synchronized. + Sun recommends using separate format instances for each thread. + If multiple threads must access a static formatter, the formatter must be + synchronized either on method or block level. + + 3 + + + + + + + + Since Java5 brought a new implementation of the Map designed for multi-threaded access, you can + perform efficient map reads without blocking other threads. + + 3 + + + + + + + + + + + + + + + Thread.notify() awakens a thread monitoring the object. If more than one thread is monitoring, then only + one is chosen. The thread chosen is arbitrary; thus its usually safer to call notifyAll() instead. + + 3 + + + + + + + + + + + + + \ No newline at end of file diff --git a/gradle/quality/pmd/category/java/performance.xml b/gradle/quality/pmd/category/java/performance.xml new file mode 100644 index 0000000..1ce2d8d --- /dev/null +++ b/gradle/quality/pmd/category/java/performance.xml @@ -0,0 +1,1006 @@ + + + + + + Rules that flag suboptimal code. + + + + + The conversion of literals to strings by concatenating them with empty strings is inefficient. + It is much better to use one of the type-specific toString() methods instead. + + 3 + + + + + + + + + + + + + + + Avoid concatenating characters as strings in StringBuffer/StringBuilder.append methods. + + 3 + + + + + + + + Instead of manually copying data between two arrays, use the efficient Arrays.copyOf or System.arraycopy method instead. + + 3 + + + + + + + + + + + + + + + The FileInputStream and FileOutputStream classes contains a finalizer method which will cause garbage + collection pauses. + See [JDK-8080225](https://bugs.openjdk.java.net/browse/JDK-8080225) for details. + + The FileReader and FileWriter constructors instantiate FileInputStream and FileOutputStream, + again causing garbage collection issues while finalizer methods are called. + + * Use `Files.newInputStream(Paths.get(fileName))` instead of `new FileInputStream(fileName)`. + * Use `Files.newOutputStream(Paths.get(fileName))` instead of `new FileOutputStream(fileName)`. + * Use `Files.newBufferedReader(Paths.get(fileName))` instead of `new FileReader(fileName)`. + * Use `Files.newBufferedWriter(Paths.get(fileName))` instead of `new FileWriter(fileName)`. + + Please note, that the `java.nio` API does not throw a `FileNotFoundException` anymore, instead + it throws a `NoSuchFileException`. If your code dealt explicitly with a `FileNotFoundException`, + then this needs to be adjusted. Both exceptions are subclasses of `IOException`, so catching + that one covers both. + + 1 + + + + + + + + + + + + + + + New objects created within loops should be checked to see if they can created outside them and reused. + + 3 + + + + + + + + Java uses the 'short' type to reduce memory usage, not to optimize calculation. In fact, the JVM does not have any + arithmetic capabilities for the short type: the JVM must convert the short into an int, do the proper calculation + and convert the int back to a short. Thus any storage gains found through use of the 'short' type may be offset by + adverse impacts on performance. + + 1 + + + + + + + + + + + + + + + Don't create instances of already existing BigInteger (BigInteger.ZERO, BigInteger.ONE) and + for Java 1.5 onwards, BigInteger.TEN and BigDecimal (BigDecimal.ZERO, BigDecimal.ONE, BigDecimal.TEN) + + 3 + + + + + + + + Avoid instantiating Boolean objects; you can reference Boolean.TRUE, Boolean.FALSE, or call Boolean.valueOf() instead. + Note that new Boolean() is deprecated since JDK 9 for that reason. + + 2 + + + + + + + + Calling new Byte() causes memory allocation that can be avoided by the static Byte.valueOf(). + It makes use of an internal cache that recycles earlier instances making it more memory efficient. + Note that new Byte() is deprecated since JDK 9 for that reason. + + 2 + + + + + + + + + + + + + + + Consecutive calls to StringBuffer/StringBuilder .append should be chained, reusing the target object. This can improve the performance + by producing a smaller bytecode, reducing overhead and improving inlining. A complete analysis can be found [here](https://github.com/pmd/pmd/issues/202#issuecomment-274349067) + + 3 + + + + + + + + Consecutively calling StringBuffer/StringBuilder.append(...) with literals should be avoided. + Since the literals are constants, they can already be combined into a single String literal and this String + can be appended in a single method call. + + 3 + + + + + + + + + + 3 + + 0) { + doSomething(); + } +} +]]> + + + + + + Avoid concatenating non-literals in a StringBuffer constructor or append() since intermediate buffers will + need to be be created and destroyed by the JVM. + + 3 + + + + + + + + Failing to pre-size a StringBuffer or StringBuilder properly could cause it to re-size many times + during runtime. This rule attempts to determine the total number the characters that are actually + passed into StringBuffer.append(), but represents a best guess "worst case" scenario. An empty + StringBuffer/StringBuilder constructor initializes the object to 16 characters. This default + is assumed if the length of the constructor can not be determined. + + 3 + + + + + + + + Calling new Integer() causes memory allocation that can be avoided by the static Integer.valueOf(). + It makes use of an internal cache that recycles earlier instances making it more memory efficient. + Note that new Integer() is deprecated since JDK 9 for that reason. + + 2 + + + + + + + + + + + + + + + Calling new Long() causes memory allocation that can be avoided by the static Long.valueOf(). + It makes use of an internal cache that recycles earlier instances making it more memory efficient. + Note that new Long() is deprecated since JDK 9 for that reason. + + 2 + + + + + + + + + + + + + + + Calls to a collection's `toArray(E[])` method should specify a target array of zero size. This allows the JVM + to optimize the memory allocation and copying as much as possible. + + Previous versions of this rule (pre PMD 6.0.0) suggested the opposite, but current JVM implementations + perform always better, when they have full control over the target array. And allocation an array via + reflection is nowadays as fast as the direct allocation. + + See also [Arrays of Wisdom of the Ancients](https://shipilev.net/blog/2016/arrays-wisdom-ancients/) + + Note: If you don't need an array of the correct type, then the simple `toArray()` method without an array + is faster, but returns only an array of type `Object[]`. + + 3 + + + + + + + + + foos = getFoos(); + +// much better; this one allows the jvm to allocate an array of the correct size and effectively skip +// the zeroing, since each array element will be overridden anyways +Foo[] fooArray = foos.toArray(new Foo[0]); + +// inefficient, the array needs to be zeroed out by the jvm before it is handed over to the toArray method +Foo[] fooArray = foos.toArray(new Foo[foos.size()]); +]]> + + + + + + Java will initialize fields with known default values so any explicit initialization of those same defaults + is redundant and results in a larger class file (approximately three additional bytecode instructions per field). + + 3 + + + + + + + + Since it passes in a literal of length 1, calls to (string).startsWith can be rewritten using (string).charAt(0) + at the expense of some readability. + + 3 + + + + + + + + + + + + + + + Calling new Short() causes memory allocation that can be avoided by the static Short.valueOf(). + It makes use of an internal cache that recycles earlier instances making it more memory efficient. + Note that new Short() is deprecated since JDK 9 for that reason. + + 2 + + + + + + + + + + + + + + + Avoid instantiating String objects; this is usually unnecessary since they are immutable and can be safely shared. + + 2 + + + + + + + + Avoid calling toString() on objects already known to be string instances; this is unnecessary. + + 3 + + + + + + + + Switch statements are intended to be used to support complex branching behaviour. Using a switch for only a few + cases is ill-advised, since switches are not as easy to understand as if-then statements. In these cases use the + if-then statement to increase code readability. + + 3 + + + + + + + + + + + + + + + + Most wrapper classes provide static conversion methods that avoid the need to create intermediate objects + just to create the primitive forms. Using these avoids the cost of creating objects that also need to be + garbage-collected later. + + 3 + + + + + + + + ArrayList is a much better Collection implementation than Vector if thread-safe operation is not required. + + 3 + + + + 0] + //AllocationExpression/ClassOrInterfaceType + [@Image='Vector' or @Image='java.util.Vector'] +]]> + + + + + + + + + + + (Arrays.asList(...)) if that is inconvenient for you (e.g. because of concurrent access). +]]> + + 3 + + + + + + + + + l= new ArrayList<>(100); + for (int i=0; i< 100; i++) { + l.add(ints[i]); + } + for (int i=0; i< 100; i++) { + l.add(a[i].toString()); // won't trigger the rule + } + } +} +]]> + + + + + + Use String.indexOf(char) when checking for the index of a single character; it executes faster. + + 3 + + + + + + + + No need to call String.valueOf to append to a string; just use the valueOf() argument directly. + + 3 + + + + + + + + The use of the '+=' operator for appending strings causes the JVM to create and use an internal StringBuffer. + If a non-trivial number of these concatenations are being used then the explicit use of a StringBuilder or + threadsafe StringBuffer is recommended to avoid this. + + 3 + + + + + + + + Use StringBuffer.length() to determine StringBuffer length rather than using StringBuffer.toString().equals("") + or StringBuffer.toString().length() == ... + + 3 + + + + + + + + + diff --git a/gradle/quality/pmd/category/java/security.xml b/gradle/quality/pmd/category/java/security.xml new file mode 100644 index 0000000..dbad352 --- /dev/null +++ b/gradle/quality/pmd/category/java/security.xml @@ -0,0 +1,65 @@ + + + + + + Rules that flag potential security flaws. + + + + + Do not use hard coded values for cryptographic operations. Please store keys outside of source code. + + 3 + + + + + + + + Do not use hard coded initialization vector in cryptographic operations. Please use a randomly generated IV. + + 3 + + + + + + diff --git a/gradle/quality/sonarqube.gradle b/gradle/quality/sonarqube.gradle new file mode 100644 index 0000000..fe66cd0 --- /dev/null +++ b/gradle/quality/sonarqube.gradle @@ -0,0 +1,37 @@ + +subprojects { + + sonarqube { + properties { + property "sonar.projectName", "${project.group} ${project.name}" + property "sonar.sourceEncoding", "UTF-8" + property "sonar.tests", "src/test/java" + property "sonar.scm.provider", "git" + property "sonar.junit.reportsPath", "build/test-results/test/" + } + } + + + tasks.withType(Pmd) { + ignoreFailures = true + reports { + xml.enabled = true + html.enabled = true + } + } + + + spotbugs { + effort = "max" + reportLevel = "low" + //includeFilter = file("findbugs-exclude.xml") + } + + tasks.withType(com.github.spotbugs.SpotBugsTask) { + ignoreFailures = true + reports { + xml.enabled = false + html.enabled = true + } + } +} \ No newline at end of file diff --git a/gradle/quality/spotbugs.gradle b/gradle/quality/spotbugs.gradle index 8e671e2..2e5b0cd 100644 --- a/gradle/quality/spotbugs.gradle +++ b/gradle/quality/spotbugs.gradle @@ -1,53 +1,15 @@ -/* + +apply plugin: 'com.github.spotbugs' + spotbugs { effort = "max" reportLevel = "low" + ignoreFailures = true } -tasks.withType(com.github.spotbugs.SpotBugsTask) { - ignoreFailures = true +spotbugsMain { reports { - xml.enabled = false - html.enabled = true + xml.getRequired().set(false) + html.getRequired().set(true) } } - -pmd { - toolVersion = '6.11.0' - ruleSets = ['category/java/bestpractices.xml'] -} - -tasks.withType(Pmd) { - ignoreFailures = true - reports { - xml.enabled = true - html.enabled = true - } -} - -checkstyle { - toolVersion = '8.26' - configFile = rootProject.file('config/checkstyle/checkstyle.xml') - ignoreFailures = true - checkstyleMain { - source = sourceSets.main.allSource - } -} - -tasks.withType(Checkstyle) { - ignoreFailures = true - reports { - xml.enabled = true - html.enabled = true - } -} - -sonarqube { - properties { - property "sonar.projectName", "${project.group} ${project.name}" - property "sonar.sourceEncoding", "UTF-8" - property "sonar.tests", "src/test/java" - property "sonar.scm.provider", "git" - } -} -*/ \ No newline at end of file diff --git a/gradle/test/junit5.gradle b/gradle/test/junit5.gradle index 10ad7d2..3a9a9dd 100644 --- a/gradle/test/junit5.gradle +++ b/gradle/test/junit5.gradle @@ -12,7 +12,7 @@ dependencies { test { useJUnitPlatform() - failFast = true + failFast = false testLogging { events 'STARTED', 'PASSED', 'FAILED', 'SKIPPED' } diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index e708b1c..c1962a7 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 33682bb..8707e8b 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-6.6.1-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.1.1-all.zip +networkTimeout=10000 zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/gradlew b/gradlew index 4f906e0..aeb74cb 100755 --- a/gradlew +++ b/gradlew @@ -1,7 +1,7 @@ -#!/usr/bin/env sh +#!/bin/sh # -# Copyright 2015 the original author or authors. +# Copyright © 2015-2021 the original authors. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -17,67 +17,98 @@ # ############################################################################## -## -## Gradle start up script for UN*X -## +# +# Gradle start up script for POSIX generated by Gradle. +# +# Important for running: +# +# (1) You need a POSIX-compliant shell to run this script. If your /bin/sh is +# noncompliant, but you have some other compliant shell such as ksh or +# bash, then to run this script, type that shell name before the whole +# command line, like: +# +# ksh Gradle +# +# Busybox and similar reduced shells will NOT work, because this script +# requires all of these POSIX shell features: +# * functions; +# * expansions «$var», «${var}», «${var:-default}», «${var+SET}», +# «${var#prefix}», «${var%suffix}», and «$( cmd )»; +# * compound commands having a testable exit status, especially «case»; +# * various built-in commands including «command», «set», and «ulimit». +# +# Important for patching: +# +# (2) This script targets any POSIX shell, so it avoids extensions provided +# by Bash, Ksh, etc; in particular arrays are avoided. +# +# The "traditional" practice of packing multiple parameters into a +# space-separated string is a well documented source of bugs and security +# problems, so this is (mostly) avoided, by progressively accumulating +# options in "$@", and eventually passing that to Java. +# +# Where the inherited environment variables (DEFAULT_JVM_OPTS, JAVA_OPTS, +# and GRADLE_OPTS) rely on word-splitting, this is performed explicitly; +# see the in-line comments for details. +# +# There are tweaks for specific operating systems such as AIX, CygWin, +# Darwin, MinGW, and NonStop. +# +# (3) This script is generated from the Groovy template +# https://github.com/gradle/gradle/blob/HEAD/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt +# within the Gradle project. +# +# You can find Gradle at https://github.com/gradle/gradle/. +# ############################################################################## # Attempt to set APP_HOME + # Resolve links: $0 may be a link -PRG="$0" -# Need this for relative symlinks. -while [ -h "$PRG" ] ; do - ls=`ls -ld "$PRG"` - link=`expr "$ls" : '.*-> \(.*\)$'` - if expr "$link" : '/.*' > /dev/null; then - PRG="$link" - else - PRG=`dirname "$PRG"`"/$link" - fi +app_path=$0 + +# Need this for daisy-chained symlinks. +while + APP_HOME=${app_path%"${app_path##*/}"} # leaves a trailing /; empty if no leading path + [ -h "$app_path" ] +do + ls=$( ls -ld "$app_path" ) + link=${ls#*' -> '} + case $link in #( + /*) app_path=$link ;; #( + *) app_path=$APP_HOME$link ;; + esac done -SAVED="`pwd`" -cd "`dirname \"$PRG\"`/" >/dev/null -APP_HOME="`pwd -P`" -cd "$SAVED" >/dev/null -APP_NAME="Gradle" -APP_BASE_NAME=`basename "$0"` - -# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' +# This is normally unused +# shellcheck disable=SC2034 +APP_BASE_NAME=${0##*/} +APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit # Use the maximum available, or set MAX_FD != -1 to use that value. -MAX_FD="maximum" +MAX_FD=maximum warn () { echo "$*" -} +} >&2 die () { echo echo "$*" echo exit 1 -} +} >&2 # OS specific support (must be 'true' or 'false'). cygwin=false msys=false darwin=false nonstop=false -case "`uname`" in - CYGWIN* ) - cygwin=true - ;; - Darwin* ) - darwin=true - ;; - MINGW* ) - msys=true - ;; - NONSTOP* ) - nonstop=true - ;; +case "$( uname )" in #( + CYGWIN* ) cygwin=true ;; #( + Darwin* ) darwin=true ;; #( + MSYS* | MINGW* ) msys=true ;; #( + NONSTOP* ) nonstop=true ;; esac CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar @@ -87,9 +118,9 @@ CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar if [ -n "$JAVA_HOME" ] ; then if [ -x "$JAVA_HOME/jre/sh/java" ] ; then # IBM's JDK on AIX uses strange locations for the executables - JAVACMD="$JAVA_HOME/jre/sh/java" + JAVACMD=$JAVA_HOME/jre/sh/java else - JAVACMD="$JAVA_HOME/bin/java" + JAVACMD=$JAVA_HOME/bin/java fi if [ ! -x "$JAVACMD" ] ; then die "ERROR: JAVA_HOME is set to an invalid directory: $JAVA_HOME @@ -98,7 +129,7 @@ Please set the JAVA_HOME variable in your environment to match the location of your Java installation." fi else - JAVACMD="java" + JAVACMD=java which java >/dev/null 2>&1 || die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH. Please set the JAVA_HOME variable in your environment to match the @@ -106,80 +137,109 @@ location of your Java installation." fi # Increase the maximum file descriptors if we can. -if [ "$cygwin" = "false" -a "$darwin" = "false" -a "$nonstop" = "false" ] ; then - MAX_FD_LIMIT=`ulimit -H -n` - if [ $? -eq 0 ] ; then - if [ "$MAX_FD" = "maximum" -o "$MAX_FD" = "max" ] ; then - MAX_FD="$MAX_FD_LIMIT" - fi - ulimit -n $MAX_FD - if [ $? -ne 0 ] ; then - warn "Could not set maximum file descriptor limit: $MAX_FD" - fi - else - warn "Could not query maximum file descriptor limit: $MAX_FD_LIMIT" - fi -fi - -# For Darwin, add options to specify how the application appears in the dock -if $darwin; then - GRADLE_OPTS="$GRADLE_OPTS \"-Xdock:name=$APP_NAME\" \"-Xdock:icon=$APP_HOME/media/gradle.icns\"" -fi - -# For Cygwin or MSYS, switch paths to Windows format before running java -if [ "$cygwin" = "true" -o "$msys" = "true" ] ; then - APP_HOME=`cygpath --path --mixed "$APP_HOME"` - CLASSPATH=`cygpath --path --mixed "$CLASSPATH"` - - JAVACMD=`cygpath --unix "$JAVACMD"` - - # We build the pattern for arguments to be converted via cygpath - ROOTDIRSRAW=`find -L / -maxdepth 1 -mindepth 1 -type d 2>/dev/null` - SEP="" - for dir in $ROOTDIRSRAW ; do - ROOTDIRS="$ROOTDIRS$SEP$dir" - SEP="|" - done - OURCYGPATTERN="(^($ROOTDIRS))" - # Add a user-defined pattern to the cygpath arguments - if [ "$GRADLE_CYGPATTERN" != "" ] ; then - OURCYGPATTERN="$OURCYGPATTERN|($GRADLE_CYGPATTERN)" - fi - # Now convert the arguments - kludge to limit ourselves to /bin/sh - i=0 - for arg in "$@" ; do - CHECK=`echo "$arg"|egrep -c "$OURCYGPATTERN" -` - CHECK2=`echo "$arg"|egrep -c "^-"` ### Determine if an option - - if [ $CHECK -ne 0 ] && [ $CHECK2 -eq 0 ] ; then ### Added a condition - eval `echo args$i`=`cygpath --path --ignore --mixed "$arg"` - else - eval `echo args$i`="\"$arg\"" - fi - i=`expr $i + 1` - done - case $i in - 0) set -- ;; - 1) set -- "$args0" ;; - 2) set -- "$args0" "$args1" ;; - 3) set -- "$args0" "$args1" "$args2" ;; - 4) set -- "$args0" "$args1" "$args2" "$args3" ;; - 5) set -- "$args0" "$args1" "$args2" "$args3" "$args4" ;; - 6) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" ;; - 7) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" "$args6" ;; - 8) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" "$args6" "$args7" ;; - 9) set -- "$args0" "$args1" "$args2" "$args3" "$args4" "$args5" "$args6" "$args7" "$args8" ;; +if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then + case $MAX_FD in #( + max*) + # In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked. + # shellcheck disable=SC3045 + MAX_FD=$( ulimit -H -n ) || + warn "Could not query maximum file descriptor limit" + esac + case $MAX_FD in #( + '' | soft) :;; #( + *) + # In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked. + # shellcheck disable=SC3045 + ulimit -n "$MAX_FD" || + warn "Could not set maximum file descriptor limit to $MAX_FD" esac fi -# Escape application args -save () { - for i do printf %s\\n "$i" | sed "s/'/'\\\\''/g;1s/^/'/;\$s/\$/' \\\\/" ; done - echo " " -} -APP_ARGS=`save "$@"` +# Collect all arguments for the java command, stacking in reverse order: +# * args from the command line +# * the main class name +# * -classpath +# * -D...appname settings +# * --module-path (only if needed) +# * DEFAULT_JVM_OPTS, JAVA_OPTS, and GRADLE_OPTS environment variables. -# Collect all arguments for the java command, following the shell quoting and substitution rules -eval set -- $DEFAULT_JVM_OPTS $JAVA_OPTS $GRADLE_OPTS "\"-Dorg.gradle.appname=$APP_BASE_NAME\"" -classpath "\"$CLASSPATH\"" org.gradle.wrapper.GradleWrapperMain "$APP_ARGS" +# For Cygwin or MSYS, switch paths to Windows format before running java +if "$cygwin" || "$msys" ; then + APP_HOME=$( cygpath --path --mixed "$APP_HOME" ) + CLASSPATH=$( cygpath --path --mixed "$CLASSPATH" ) + + JAVACMD=$( cygpath --unix "$JAVACMD" ) + + # Now convert the arguments - kludge to limit ourselves to /bin/sh + for arg do + if + case $arg in #( + -*) false ;; # don't mess with options #( + /?*) t=${arg#/} t=/${t%%/*} # looks like a POSIX filepath + [ -e "$t" ] ;; #( + *) false ;; + esac + then + arg=$( cygpath --path --ignore --mixed "$arg" ) + fi + # Roll the args list around exactly as many times as the number of + # args, so each arg winds up back in the position where it started, but + # possibly modified. + # + # NB: a `for` loop captures its iteration list before it begins, so + # changing the positional parameters here affects neither the number of + # iterations, nor the values presented in `arg`. + shift # remove old arg + set -- "$@" "$arg" # push replacement arg + done +fi + + +# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. +DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' + +# Collect all arguments for the java command; +# * $DEFAULT_JVM_OPTS, $JAVA_OPTS, and $GRADLE_OPTS can contain fragments of +# shell script including quotes and variable substitutions, so put them in +# double quotes to make sure that they get re-expanded; and +# * put everything else in single quotes, so that it's not re-expanded. + +set -- \ + "-Dorg.gradle.appname=$APP_BASE_NAME" \ + -classpath "$CLASSPATH" \ + org.gradle.wrapper.GradleWrapperMain \ + "$@" + +# Stop when "xargs" is not available. +if ! command -v xargs >/dev/null 2>&1 +then + die "xargs is not available" +fi + +# Use "xargs" to parse quoted args. +# +# With -n1 it outputs one arg per line, with the quotes and backslashes removed. +# +# In Bash we could simply go: +# +# readarray ARGS < <( xargs -n1 <<<"$var" ) && +# set -- "${ARGS[@]}" "$@" +# +# but POSIX shell has neither arrays nor command substitution, so instead we +# post-process each arg (as a line of input to sed) to backslash-escape any +# character that might be a shell metacharacter, then use eval to reverse +# that process (while maintaining the separation between arguments), and wrap +# the whole thing up as a single "set" statement. +# +# This will of course break if any of these variables contains a newline or +# an unmatched quote. +# + +eval "set -- $( + printf '%s\n' "$DEFAULT_JVM_OPTS $JAVA_OPTS $GRADLE_OPTS" | + xargs -n1 | + sed ' s~[^-[:alnum:]+,./:=@_]~\\&~g; ' | + tr '\n' ' ' + )" '"$@"' exec "$JAVACMD" "$@" diff --git a/gradlew.bat b/gradlew.bat index ac1b06f..6689b85 100644 --- a/gradlew.bat +++ b/gradlew.bat @@ -14,7 +14,7 @@ @rem limitations under the License. @rem -@if "%DEBUG%" == "" @echo off +@if "%DEBUG%"=="" @echo off @rem ########################################################################## @rem @rem Gradle startup script for Windows @@ -25,7 +25,8 @@ if "%OS%"=="Windows_NT" setlocal set DIRNAME=%~dp0 -if "%DIRNAME%" == "" set DIRNAME=. +if "%DIRNAME%"=="" set DIRNAME=. +@rem This is normally unused set APP_BASE_NAME=%~n0 set APP_HOME=%DIRNAME% @@ -40,7 +41,7 @@ if defined JAVA_HOME goto findJavaFromJavaHome set JAVA_EXE=java.exe %JAVA_EXE% -version >NUL 2>&1 -if "%ERRORLEVEL%" == "0" goto execute +if %ERRORLEVEL% equ 0 goto execute echo. echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH. @@ -75,13 +76,15 @@ set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar :end @rem End local scope for the variables with windows NT shell -if "%ERRORLEVEL%"=="0" goto mainEnd +if %ERRORLEVEL% equ 0 goto mainEnd :fail rem Set variable GRADLE_EXIT_CONSOLE if you need the _script_ return code instead of rem the _cmd.exe /c_ return code! -if not "" == "%GRADLE_EXIT_CONSOLE%" exit 1 -exit /b 1 +set EXIT_CODE=%ERRORLEVEL% +if %EXIT_CODE% equ 0 set EXIT_CODE=1 +if not ""=="%GRADLE_EXIT_CONSOLE%" exit %EXIT_CODE% +exit /b %EXIT_CODE% :mainEnd if "%OS%"=="Windows_NT" endlocal diff --git a/settings.gradle b/settings.gradle index 89f4110..cb29c87 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1 +1,49 @@ -rootProject.name = name +pluginManagement { + repositories { + mavenLocal() + mavenCentral { + metadataSources { + mavenPom() + artifact() + ignoreGradleMetadataRedirection() + } + } + gradlePluginPortal() + } +} + +dependencyResolutionManagement { + versionCatalogs { + libs { + version('gradle', '8.1.1') + version('groovy', '3.0.10') + version('junit', '5.9.2') + library('junit-jupiter-api', 'org.junit.jupiter', 'junit-jupiter-api').versionRef('junit') + library('junit-jupiter-params', 'org.junit.jupiter', 'junit-jupiter-params').versionRef('junit') + library('junit-jupiter-engine', 'org.junit.jupiter', 'junit-jupiter-engine').versionRef('junit') + library('hamcrest', 'org.hamcrest', 'hamcrest-library').version('2.2') + library('junit4', 'junit', 'junit').version('4.13.2') + library('javax.inject', 'org.xbib', 'javax-inject').version('1') + library('guava', 'org.xbib', 'guava').version('30.1') + library('guava.testlib', 'com.google.guava', 'guava-testlib').version('30.1-jre') + } + } +} + +/* +gradle.wrapper.version = 6.6.1 +javax-inject.version = 1 +guava.version = 30.1 +# test +junit.version = 5.7.2 +junit4.version = 4.13.2 +log4j.version = 2.14.1 + + api "org.xbib:javax-inject:${project.property('javax-inject.version')}" + api "org.xbib:guava:${project.property('guava.version')}" + testImplementation "junit:junit:${project.property('junit4.version')}" + // Helper for com.google.common.testing.GcFinalization, com.google.common.testing.EqualsTester + testImplementation "com.google.guava:guava-testlib:30.1-jre" + + + */ \ No newline at end of file diff --git a/src/main/java/com/google/inject/name/NamedImpl.java b/src/main/java/com/google/inject/name/NamedImpl.java index 265d74a..d23a69b 100644 --- a/src/main/java/com/google/inject/name/NamedImpl.java +++ b/src/main/java/com/google/inject/name/NamedImpl.java @@ -41,6 +41,6 @@ class NamedImpl implements Named { @Override public String toString() { - return "@" + Named.class.getName() + "(value=" + Annotations.memberValueString(value) + ")"; + return "@" + Named.class.getName() + "(" + Annotations.memberValueString(value) + ")"; } } diff --git a/src/main/java/com/google/inject/spi/ElementSource.java b/src/main/java/com/google/inject/spi/ElementSource.java index 754f054..575ad02 100644 --- a/src/main/java/com/google/inject/spi/ElementSource.java +++ b/src/main/java/com/google/inject/spi/ElementSource.java @@ -1,9 +1,6 @@ package com.google.inject.spi; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.inject.internal.util.StackTraceElements; -import com.google.inject.internal.util.StackTraceElements.InMemoryStackTraceElement; import java.util.List; /** @@ -13,21 +10,6 @@ import java.util.List; * defines the Guice {@link Element element}. For example, if the element is created from a method * annotated by {@literal @Provides}, the declaring source of element would be the method itself. * - *

The {@link #getStackTrace()} refers to the sequence of calls ends at one of {@link - * com.google.inject.Binder} {@code bindXXX()} methods and eventually defines the element. Note that - * {@link #getStackTrace()} lists {@link StackTraceElement StackTraceElements} in reverse - * chronological order. The first element (index zero) is the last method call and the last element - * is the first method invocation. By default, the stack trace is not collected. The default - * behavior can be changed by setting the {@code guice_include_stack_traces} flag value. The value - * can be either {@code OFF}, {@code ONLY_FOR_DECLARING_SOURCE} or {@code COMPLETE}. Note that - * collecting stack traces for every binding can cause a performance hit when the injector is - * created. - * - *

The sequence of class names of {@link com.google.inject.Module modules} involved in the - * element creation can be retrieved by {@link #getModuleClassNames()}. Similar to {@link - * #getStackTrace()}, the order is reverse chronological. The first module (index 0) is the module - * that installs the {@link Element element}. The last module is the root module. - * *

In order to support the cases where a Guice {@link Element element} is created from another * Guice {@link Element element} (original) (e.g., by {@link Element#applyTo}), it also provides a * reference to the original element source ({@link #getOriginalElementSource()}). @@ -54,12 +36,6 @@ public final class ElementSource { /** The {@link ModuleSource source} of module creates the element. */ final ModuleSource moduleSource; - /** - * The partial call stack that starts at the last module {@link Module#Configure(Binder) - * configure(Binder)} call. The value is empty if stack trace collection is off. - */ - final InMemoryStackTraceElement[] partialCallStack; - /** * Refers to a single location in source code that causes the element creation. It can be any * object such as {@link Constructor}, {@link Method}, {@link Field}, {@link StackTraceElement}, @@ -78,24 +54,19 @@ public final class ElementSource { * any), otherwise {@code null}. * @param declaringSource the source (in)directly declared the element. * @param moduleSource the moduleSource when the element is bound - * @param partialCallStack the partial call stack from the top module to where the element is - * bound */ ElementSource( ElementSource originalSource, boolean trustedOriginalSource, Object declaringSource, ModuleSource moduleSource, - StackTraceElement[] partialCallStack, ModuleAnnotatedMethodScanner scanner) { Preconditions.checkNotNull(declaringSource, "declaringSource cannot be null."); Preconditions.checkNotNull(moduleSource, "moduleSource cannot be null."); - Preconditions.checkNotNull(partialCallStack, "partialCallStack cannot be null."); this.originalElementSource = originalSource; this.trustedOriginalElementSource = trustedOriginalSource; this.declaringSource = declaringSource; this.moduleSource = moduleSource; - this.partialCallStack = StackTraceElements.convertToInMemoryStackTraceElement(partialCallStack); this.scanner = scanner; } @@ -127,60 +98,6 @@ public final class ElementSource { return moduleSource.getModuleClassNames(); } - /** - * Returns the position of {@link com.google.inject.Module#configure configure(Binder)} method - * call in the {@link #getStackTrace stack trace} for modules that their classes returned by - * {@link #getModuleClassNames}. For example, if the stack trace looks like the following: - * - *

    - *
  1. {@code Binder.bind()} - *
  2. {@code ModuleTwo.configure()} - *
  3. {@code Binder.install()} - *
  4. {@code ModuleOne.configure()} - *
  5. {@code theRest(). - *
- * - *

1 and 3 are returned. - * - *

In the cases where stack trace is not available (i.e., the stack trace was not collected), - * it returns -1 for all module positions. - */ - public List getModuleConfigurePositionsInStackTrace() { - int size = moduleSource.size(); - Integer[] positions = new Integer[size]; - int chunkSize = partialCallStack.length; - positions[0] = chunkSize - 1; - ModuleSource current = moduleSource; - for (int cursor = 1; cursor < size; cursor++) { - chunkSize = current.getPartialCallStackSize(); - positions[cursor] = positions[cursor - 1] + chunkSize; - current = current.getParent(); - } - return ImmutableList.copyOf(positions); - } - - /** - * Returns the sequence of method calls that ends at one of {@link com.google.inject.Binder} - * {@code bindXXX()} methods and eventually defines the element. Note that {@link #getStackTrace} - * lists {@link StackTraceElement StackTraceElements} in reverse chronological order. The first - * element (index zero) is the last method call and the last element is the first method - * invocation. In the cases where stack trace is not available (i.e.,the stack trace was not - * collected), it returns an empty array. - */ - public StackTraceElement[] getStackTrace() { - int modulesCallStackSize = moduleSource.getStackTraceSize(); - int chunkSize = partialCallStack.length; - int size = moduleSource.getStackTraceSize() + chunkSize; - StackTraceElement[] callStack = new StackTraceElement[size]; - System.arraycopy( - StackTraceElements.convertToStackTraceElement(partialCallStack), - 0, - callStack, - 0, - chunkSize); - System.arraycopy(moduleSource.getStackTrace(), 0, callStack, chunkSize, modulesCallStackSize); - return callStack; - } /** Returns {@code getDeclaringSource().toString()} value. */ @Override diff --git a/src/main/java/com/google/inject/spi/Elements.java b/src/main/java/com/google/inject/spi/Elements.java index 5f0b07d..7d49d6b 100644 --- a/src/main/java/com/google/inject/spi/Elements.java +++ b/src/main/java/com/google/inject/spi/Elements.java @@ -603,23 +603,13 @@ public final class Elements { } private ModuleSource getModuleSource(Class module) { - StackTraceElement[] partialCallStack; - if (getIncludeStackTraceOption() == IncludeStackTraceOption.COMPLETE) { - partialCallStack = getPartialCallStack(new Throwable().getStackTrace()); - } else { - partialCallStack = new StackTraceElement[0]; - } if (moduleSource == null) { - return new ModuleSource(module, partialCallStack, permitMapConstruction.getPermitMap()); + return new ModuleSource(module, permitMapConstruction.getPermitMap()); } - return moduleSource.createChild(module, partialCallStack); + return moduleSource.createChild(module); } private ElementSource getElementSource() { - // Full call stack - StackTraceElement[] callStack = null; - // The call stack starts from current top module configure and ends at this method caller - StackTraceElement[] partialCallStack = new StackTraceElement[0]; // The element original source ElementSource originalSource = null; // The element declaring source @@ -628,21 +618,10 @@ public final class Elements { originalSource = (ElementSource) declaringSource; declaringSource = originalSource.getDeclaringSource(); } - IncludeStackTraceOption stackTraceOption = getIncludeStackTraceOption(); - if (stackTraceOption == IncludeStackTraceOption.COMPLETE - || (stackTraceOption == IncludeStackTraceOption.ONLY_FOR_DECLARING_SOURCE - && declaringSource == null)) { - callStack = new Throwable().getStackTrace(); - } - if (stackTraceOption == IncludeStackTraceOption.COMPLETE) { - partialCallStack = getPartialCallStack(callStack); - } if (declaringSource == null) { - // So 'source' and 'originalSource' are null otherwise declaringSource has some value - if (stackTraceOption == IncludeStackTraceOption.COMPLETE - || stackTraceOption == IncludeStackTraceOption.ONLY_FOR_DECLARING_SOURCE) { - // With the above conditions and assignments 'callStack' is non-null - StackTraceElement callingSource = sourceProvider.get(callStack); + IncludeStackTraceOption stackTraceOption = getIncludeStackTraceOption(); + if (stackTraceOption == IncludeStackTraceOption.ONLY_FOR_DECLARING_SOURCE) { + StackTraceElement callingSource = sourceProvider.get(new Throwable().getStackTrace()); // If we've traversed past all reasonable sources and into our internal code, then we // don't know the source. if (callingSource @@ -653,37 +632,14 @@ public final class Elements { } else { declaringSource = callingSource; } - } else { // or if (stackTraceOption == IncludeStackTraceOptions.OFF) + } else { // As neither 'declaring source' nor 'call stack' is available use 'module source' declaringSource = sourceProvider.getFromClassNames(moduleSource.getModuleClassNames()); } } // Build the binding call stack return new ElementSource( - originalSource, - trustedSource, - declaringSource, - moduleSource, - partialCallStack, - scannerSource); - } - - /** - * Removes the {@link #moduleSource} call stack from the beginning of current call stack. It - * also removes the last two elements in order to make {@link #install(Module)} the last call in - * the call stack. - */ - private StackTraceElement[] getPartialCallStack(StackTraceElement[] callStack) { - int toSkip = 0; - if (moduleSource != null) { - toSkip = moduleSource.getStackTraceSize(); - } - // -1 for skipping 'getModuleSource' and 'getElementSource' calls - int chunkSize = callStack.length - toSkip - 1; - - StackTraceElement[] partialCallStack = new StackTraceElement[chunkSize]; - System.arraycopy(callStack, 1, partialCallStack, 0, chunkSize); - return partialCallStack; + originalSource, trustedSource, declaringSource, moduleSource, scannerSource); } /** Returns if the binder is in the module scanning phase. */ diff --git a/src/main/java/com/google/inject/spi/ModuleSource.java b/src/main/java/com/google/inject/spi/ModuleSource.java index 2e2a886..e0a595e 100644 --- a/src/main/java/com/google/inject/spi/ModuleSource.java +++ b/src/main/java/com/google/inject/spi/ModuleSource.java @@ -3,14 +3,11 @@ package com.google.inject.spi; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.inject.Module; -import com.google.inject.internal.util.StackTraceElements; -import com.google.inject.internal.util.StackTraceElements.InMemoryStackTraceElement; import java.util.List; /** * Associated to a {@link Module module}, provides the module class name, the parent module {@link - * ModuleSource source}, and the call stack that ends just before the module {@link - * Module#configure(Binder) configure(Binder)} method invocation. + * ModuleSource source}, and the call stack that ends just before the method invocation. */ final class ModuleSource { @@ -29,27 +26,14 @@ final class ModuleSource { */ private final BindingSourceRestriction.PermitMap permitMap; - /** - * The chunk of call stack that starts from the parent module {@link Module#configure(Binder) - * configure(Binder)} call and ends just before the module {@link Module#configure(Binder) - * configure(Binder)} method invocation. For a module without a parent module the chunk starts - * from the bottom of call stack. The array is non-empty if stack trace collection is on. - */ - private final InMemoryStackTraceElement[] partialCallStack; - /** * Creates a new {@link ModuleSource} with a {@literal null} parent. * * @param moduleClass the corresponding module - * @param partialCallStack the chunk of call stack that starts from the parent module {@link - * Module#configure(Binder) configure(Binder)} call and ends just before the module {@link - * Module#configure(Binder) configure(Binder)} method invocation */ - ModuleSource( - Class moduleClass, - StackTraceElement[] partialCallStack, + ModuleSource(Class moduleClass, BindingSourceRestriction.PermitMap permitMap) { - this(null, moduleClass, partialCallStack, permitMap); + this(null, moduleClass, permitMap); } /** @@ -57,20 +41,14 @@ final class ModuleSource { * * @param parent the parent module {@link ModuleSource source} * @param moduleClass the corresponding module - * @param partialCallStack the chunk of call stack that starts from the parent module {@link - * Module#configure(Binder) configure(Binder)} call and ends just before the module {@link - * Module#configure(Binder) configure(Binder)} method invocation */ private ModuleSource( ModuleSource parent, Class moduleClass, - StackTraceElement[] partialCallStack, BindingSourceRestriction.PermitMap permitMap) { Preconditions.checkNotNull(moduleClass, "module cannot be null."); - Preconditions.checkNotNull(partialCallStack, "partialCallStack cannot be null."); this.parent = parent; this.moduleClassName = moduleClass.getName(); - this.partialCallStack = StackTraceElements.convertToInMemoryStackTraceElement(partialCallStack); this.permitMap = permitMap; } @@ -83,31 +61,13 @@ final class ModuleSource { return moduleClassName; } - /** - * Returns the chunk of call stack that starts from the parent module {@link - * Module#configure(Binder) configure(Binder)} call and ends just before the module {@link - * Module#configure(Binder) configure(Binder)} method invocation. The return array is non-empty - * only if stack trace collection is on. - */ - StackTraceElement[] getPartialCallStack() { - return StackTraceElements.convertToStackTraceElement(partialCallStack); - } - - /** Returns the size of partial call stack if stack trace collection is on otherwise zero. */ - int getPartialCallStackSize() { - return partialCallStack.length; - } - /** * Creates and returns a child {@link ModuleSource} corresponding to the {@link Module module}. * * @param moduleClass the corresponding module - * @param partialCallStack the chunk of call stack that starts from the parent module {@link - * Module#configure(Binder) configure(Binder)} call and ends just before the module {@link - * Module#configure(Binder) configure(Binder)} method invocation */ - ModuleSource createChild(Class moduleClass, StackTraceElement[] partialCallStack) { - return new ModuleSource(this, moduleClass, partialCallStack, permitMap); + ModuleSource createChild(Class moduleClass) { + return new ModuleSource(this, moduleClass, permitMap); } /** Returns the parent module {@link ModuleSource source}. */ @@ -142,38 +102,6 @@ final class ModuleSource { return parent.size() + 1; } - /** - * Returns the size of call stack that ends just before the module {@link Module#configure(Binder) - * configure(Binder)} method invocation (see {@link #getStackTrace()}). - */ - int getStackTraceSize() { - if (parent == null) { - return partialCallStack.length; - } - return parent.getStackTraceSize() + partialCallStack.length; - } - - /** - * Returns the full call stack that ends just before the module {@link Module#configure(Binder) - * configure(Binder)} method invocation. The return array is non-empty if stack trace collection - * on. - */ - StackTraceElement[] getStackTrace() { - int stackTraceSize = getStackTraceSize(); - StackTraceElement[] callStack = new StackTraceElement[stackTraceSize]; - int cursor = 0; - ModuleSource current = this; - while (current != null) { - StackTraceElement[] chunk = - StackTraceElements.convertToStackTraceElement(current.partialCallStack); - int chunkSize = chunk.length; - System.arraycopy(chunk, 0, callStack, cursor, chunkSize); - current = current.parent; - cursor = cursor + chunkSize; - } - return callStack; - } - /** Returns the permit map created by the binder that installed this module. */ BindingSourceRestriction.PermitMap getPermitMap() { return permitMap; diff --git a/src/test/java/com/google/inject/BinderTest.java b/src/test/java/com/google/inject/BinderTest.java index 67ecd19..a22367d 100644 --- a/src/test/java/com/google/inject/BinderTest.java +++ b/src/test/java/com/google/inject/BinderTest.java @@ -124,9 +124,10 @@ public class BinderTest { } catch (CreationException e) { assertEquals(1, e.getErrorMessages().size()); assertContains(e.getMessage(), - "No implementation for java.lang.Runnable was bound.", - "for field at " + NeedsRunnable.class.getName(), ".runnable(", - "at " + getClass().getName(), getDeclaringSourcePart(getClass())); + "No implementation for Runnable was bound", + "for field runnable", + "at BinderTest", + getDeclaringSourcePart(getClass())); } } @@ -440,7 +441,7 @@ public class BinderTest { fail(); } catch (CreationException expected) { assertContains(expected.getMessage(), - "1) Binding to Provider is not allowed.", + "Binding to Provider is not allowed", "at " + BinderTest.class.getName(), getDeclaringSourcePart(getClass())); } } diff --git a/src/test/java/com/google/inject/BoundInstanceInjectionTest.java b/src/test/java/com/google/inject/BoundInstanceInjectionTest.java index dbf0f1a..a356cd6 100644 --- a/src/test/java/com/google/inject/BoundInstanceInjectionTest.java +++ b/src/test/java/com/google/inject/BoundInstanceInjectionTest.java @@ -8,10 +8,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import com.google.inject.name.Named; - -import org.junit.Test; import java.lang.annotation.Retention; import java.lang.annotation.Target; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.junit.Test; public class BoundInstanceInjectionTest { @@ -19,80 +20,19 @@ public class BoundInstanceInjectionTest { public void testInstancesAreInjected() throws CreationException { final O o = new O(); - Injector injector = Guice.createInjector(new AbstractModule() { - protected void configure() { - bind(O.class).toInstance(o); - bind(int.class).toInstance(5); - } - }); + Injector injector = + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(O.class).toInstance(o); + bind(int.class).toInstance(5); + } + }); assertEquals(5, o.fromMethod); } - @Test - public void testProvidersAreInjected() throws CreationException { - Injector injector = Guice.createInjector(new AbstractModule() { - protected void configure() { - bind(O.class).toProvider(new Provider() { - @Inject - int i; - - public O get() { - O o = new O(); - o.setInt(i); - return o; - } - }); - bind(int.class).toInstance(5); - } - }); - - assertEquals(5, injector.getInstance(O.class).fromMethod); - } - - @Test - public void testMalformedInstance() { - try { - Guice.createInjector(new AbstractModule() { - protected void configure() { - bind(Object.class).toInstance(new MalformedInjectable()); - } - }); - fail(); - } catch (CreationException expected) { - // disabled because of double message - /*Asserts.assertContains(expected.getMessage(), - MalformedInjectable.class.getName(), - ".doublyAnnotated() has more than one ", - "annotation annotated with @BindingAnnotation: ", - Named.class.getName() + " and " + Another.class.getName());*/ - } - } - - @Test - public void testMalformedProvider() { - try { - Guice.createInjector(new AbstractModule() { - protected void configure() { - bind(String.class).toProvider(new MalformedProvider()); - } - }); - fail(); - } catch (CreationException expected) { - Asserts.assertContains(expected.getMessage(), - MalformedProvider.class.getName(), - ".doublyAnnotated() has more than one ", - "annotation annotated with @BindingAnnotation: ", - Named.class.getName() + " and " + Another.class.getName()); - } - } - - @BindingAnnotation - @Target({FIELD, PARAMETER, METHOD}) - @Retention(RUNTIME) - public @interface Another { - } - static class O { int fromMethod; @@ -102,19 +42,90 @@ public class BoundInstanceInjectionTest { } } + @Test + public void testProvidersAreInjected() throws CreationException { + Injector injector = + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(O.class) + .toProvider( + new Provider() { + @Inject int i; + + @Override + public O get() { + O o = new O(); + o.setInt(i); + return o; + } + }); + bind(int.class).toInstance(5); + } + }); + + assertEquals(5, injector.getInstance(O.class).fromMethod); + } + + @Test + public void testMalformedInstance() { + try { + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(Object.class).toInstance(new MalformedInjectable()); + } + }); + fail(); + } catch (CreationException expected) { + Logger.getAnonymousLogger().log(Level.INFO, expected.getMessage()); + Asserts.assertContains( + expected.getMessage(), + "BoundInstanceInjectionTest$MalformedInjectable.doublyAnnotated() has more than one ", + "annotation annotated with @BindingAnnotation: ", + "Named and BoundInstanceInjectionTest$Another"); + } + } + + @Test + public void testMalformedProvider() { + try { + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(String.class).toProvider(new MalformedProvider()); + } + }); + fail(); + } catch (CreationException expected) { + Asserts.assertContains( + expected.getMessage(), + "BoundInstanceInjectionTest$MalformedProvider.doublyAnnotated() has more than one ", + "annotation annotated with @BindingAnnotation: ", + "Named and BoundInstanceInjectionTest$Another"); + } + } + static class MalformedInjectable { @Inject - void doublyAnnotated(@Named("a") @Another String unused) { - } + void doublyAnnotated(@Named("a") @Another String unused) {} } static class MalformedProvider implements Provider { @Inject - void doublyAnnotated(@Named("a") @Another String s) { - } + void doublyAnnotated(@Named("a") @Another String s) {} + @Override public String get() { return "a"; } } + + @BindingAnnotation + @Target({FIELD, PARAMETER, METHOD}) + @Retention(RUNTIME) + public @interface Another {} } diff --git a/src/test/java/com/google/inject/CircularDependencyTest.java b/src/test/java/com/google/inject/CircularDependencyTest.java index 4aece99..3185995 100644 --- a/src/test/java/com/google/inject/CircularDependencyTest.java +++ b/src/test/java/com/google/inject/CircularDependencyTest.java @@ -184,6 +184,7 @@ public class CircularDependencyTest { } } + @Test public void testDisabledCircularDependency() { try { Guice.createInjector( diff --git a/src/test/java/com/google/inject/internal/WeakKeySetTest.java b/src/test/java/com/google/inject/internal/WeakKeySetTest.java index fb9cae1..fa44493 100644 --- a/src/test/java/com/google/inject/internal/WeakKeySetTest.java +++ b/src/test/java/com/google/inject/internal/WeakKeySetTest.java @@ -2,9 +2,9 @@ package com.google.inject.internal; import static com.google.inject.Asserts.awaitClear; import static com.google.inject.Asserts.awaitFullGc; -import static com.google.inject.internal.WeakKeySetUtils.assertBlacklisted; +import static com.google.inject.internal.WeakKeySetUtils.assertBanned; import static com.google.inject.internal.WeakKeySetUtils.assertInSet; -import static com.google.inject.internal.WeakKeySetUtils.assertNotBlacklisted; +import static com.google.inject.internal.WeakKeySetUtils.assertNotBanned; import static com.google.inject.internal.WeakKeySetUtils.assertNotInSet; import static com.google.inject.internal.WeakKeySetUtils.assertSourceNotInSet; import static org.junit.Assert.assertNotNull; @@ -28,18 +28,18 @@ import com.google.inject.spi.ScopeBinding; import com.google.inject.spi.StaticInjectionRequest; import com.google.inject.spi.TypeConverterBinding; import com.google.inject.spi.TypeListenerBinding; - -import org.junit.Before; -import org.junit.Test; import java.lang.annotation.Annotation; import java.lang.ref.WeakReference; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; +import org.junit.Before; +import org.junit.Test; /** * Tests for {@link WeakKeySet}. - *

+ * * Multibinding specific tests can be found in MultibinderTest and MapBinderTest. */ public class WeakKeySetTest { @@ -47,22 +47,22 @@ public class WeakKeySetTest { private WeakKeySet set; @Before - public void setUp() { + public void setUp() throws Exception { set = new WeakKeySet(new Object()); } @Test public void testEviction() { - TestState state = new TestState(); + TestInjectorBindingData bindingData = new TestInjectorBindingData(); Key key = Key.get(Integer.class); Object source = new Object(); - WeakReference> weakKeyRef = new WeakReference>(key); + WeakReference> weakKeyRef = new WeakReference<>(key); - set.add(key, state, source); + set.add(key, bindingData, source); assertInSet(set, key, 1, source); - state = null; + bindingData = null; awaitFullGc(); @@ -75,16 +75,16 @@ public class WeakKeySetTest { @Test public void testEviction_nullSource() { - TestState state = new TestState(); + TestInjectorBindingData bindingData = new TestInjectorBindingData(); Key key = Key.get(Integer.class); Object source = null; - WeakReference> weakKeyRef = new WeakReference>(key); + WeakReference> weakKeyRef = new WeakReference<>(key); - set.add(key, state, source); + set.add(key, bindingData, source); assertInSet(set, key, 1, source); - state = null; + bindingData = null; awaitFullGc(); @@ -97,42 +97,43 @@ public class WeakKeySetTest { @Test public void testEviction_keyOverlap_2x() { - TestState state1 = new TestState(); - TestState state2 = new TestState(); + TestInjectorBindingData bindingData1 = new TestInjectorBindingData(); + TestInjectorBindingData bindingData2 = new TestInjectorBindingData(); Key key1 = Key.get(Integer.class); Key key2 = Key.get(Integer.class); Object source1 = new Object(); Object source2 = new Object(); - set.add(key1, state1, source1); + set.add(key1, bindingData1, source1); assertInSet(set, key1, 1, source1); - set.add(key2, state2, source2); + set.add(key2, bindingData2, source2); assertInSet(set, key2, 2, source1, source2); - WeakReference> weakKey1Ref = new WeakReference>(key1); - WeakReference> weakKey2Ref = new WeakReference>(key2); - WeakReference weakSource1Ref = new WeakReference(source1); - WeakReference weakSource2Ref = new WeakReference(source2); + WeakReference> weakKey1Ref = new WeakReference<>(key1); + WeakReference> weakKey2Ref = new WeakReference<>(key2); + WeakReference weakSource1Ref = new WeakReference<>(source1); + WeakReference weakSource2Ref = new WeakReference<>(source2); Key key = key1 = key2 = Key.get(Integer.class); - state1 = null; + bindingData1 = null; awaitFullGc(); assertSourceNotInSet(set, key, source1); assertInSet(set, key, 1, source2); - source1 = source2 = null; + // Clear source1 and source2 fields so the objects can be GCed. + Object unused = source1 = source2 = null; awaitClear(weakSource1Ref); // Key1 will be referenced as the key in the sources backingSet and won't be // GC'd. - // Should not be GC'd until state2 goes away. + // Should not be GC'd until bindingData2 goes away. assertNotNull(weakSource2Ref.get()); - state2 = null; + bindingData2 = null; awaitFullGc(); @@ -146,28 +147,29 @@ public class WeakKeySetTest { @Test public void testNoEviction_keyOverlap_2x() { - TestState state1 = new TestState(); - TestState state2 = new TestState(); + TestInjectorBindingData bindingData1 = new TestInjectorBindingData(); + TestInjectorBindingData bindingData2 = new TestInjectorBindingData(); Key key1 = Key.get(Integer.class); Key key2 = Key.get(Integer.class); Object source1 = new Object(); Object source2 = new Object(); - set.add(key1, state1, source1); + set.add(key1, bindingData1, source1); assertInSet(set, key1, 1, source1); - set.add(key2, state2, source2); + set.add(key2, bindingData2, source2); assertInSet(set, key2, 2, source1, source2); - WeakReference> weakKey1Ref = new WeakReference>(key1); - WeakReference> weakKey2Ref = new WeakReference>(key2); + WeakReference> weakKey1Ref = new WeakReference<>(key1); + WeakReference> weakKey2Ref = new WeakReference<>(key2); Key key = key1 = key2 = Key.get(Integer.class); awaitFullGc(); assertInSet(set, key, 2, source1, source2); - // Ensure the keys don't get GC'd when states are still referenced. key1 will be present in the + // Ensure the keys don't get GC'd when InjectorBindingData objects are still referenced. key1 + // will be present in the // as the map key but key2 could be GC'd if the implementation does something wrong. assertNotNull(weakKey1Ref.get()); assertNotNull(weakKey2Ref.get()); @@ -175,26 +177,26 @@ public class WeakKeySetTest { @Test public void testEviction_keyAndSourceOverlap_null() { - TestState state1 = new TestState(); - TestState state2 = new TestState(); + TestInjectorBindingData bindingData1 = new TestInjectorBindingData(); + TestInjectorBindingData bindingData2 = new TestInjectorBindingData(); Key key1 = Key.get(Integer.class); Key key2 = Key.get(Integer.class); Object source = null; - set.add(key1, state1, source); + set.add(key1, bindingData1, source); assertInSet(set, key1, 1, source); - set.add(key2, state2, source); + set.add(key2, bindingData2, source); // Same source so still only one value. assertInSet(set, key2, 1, source); assertInSet(set, key1, 1, source); - WeakReference> weakKey1Ref = new WeakReference>(key1); - WeakReference> weakKey2Ref = new WeakReference>(key2); - WeakReference weakSourceRef = new WeakReference(source); + WeakReference> weakKey1Ref = new WeakReference<>(key1); + WeakReference> weakKey2Ref = new WeakReference<>(key2); + WeakReference weakSourceRef = new WeakReference<>(source); Key key = key1 = key2 = Key.get(Integer.class); - state1 = null; + bindingData1 = null; awaitFullGc(); // Should still have a single source. @@ -206,7 +208,7 @@ public class WeakKeySetTest { // Key1 will be referenced as the key in the sources backingSet and won't be // GC'd. - state2 = null; + bindingData2 = null; awaitFullGc(); assertNotInSet(set, key); @@ -219,25 +221,25 @@ public class WeakKeySetTest { @Test public void testEviction_keyAndSourceOverlap_nonNull() { - TestState state1 = new TestState(); - TestState state2 = new TestState(); + TestInjectorBindingData bindingData1 = new TestInjectorBindingData(); + TestInjectorBindingData bindingData2 = new TestInjectorBindingData(); Key key1 = Key.get(Integer.class); Key key2 = Key.get(Integer.class); Object source = new Object(); - set.add(key1, state1, source); + set.add(key1, bindingData1, source); assertInSet(set, key1, 1, source); - set.add(key2, state2, source); + set.add(key2, bindingData2, source); // Same source so still only one value. assertInSet(set, key2, 1, source); - WeakReference> weakKey1Ref = new WeakReference>(key1); - WeakReference> weakKey2Ref = new WeakReference>(key2); - WeakReference weakSourceRef = new WeakReference(source); + WeakReference> weakKey1Ref = new WeakReference<>(key1); + WeakReference> weakKey2Ref = new WeakReference<>(key2); + WeakReference weakSourceRef = new WeakReference<>(source); Key key = key1 = key2 = Key.get(Integer.class); - state1 = null; + bindingData1 = null; awaitFullGc(); @@ -252,7 +254,7 @@ public class WeakKeySetTest { // Key1 will be referenced as the key in the sources backingSet and won't be // GC'd. - state2 = null; + bindingData2 = null; awaitFullGc(); @@ -266,9 +268,9 @@ public class WeakKeySetTest { @Test public void testEviction_keyOverlap_3x() { - TestState state1 = new TestState(); - TestState state2 = new TestState(); - TestState state3 = new TestState(); + TestInjectorBindingData bindingData1 = new TestInjectorBindingData(); + TestInjectorBindingData bindingData2 = new TestInjectorBindingData(); + TestInjectorBindingData bindingData3 = new TestInjectorBindingData(); Key key1 = Key.get(Integer.class); Key key2 = Key.get(Integer.class); Key key3 = Key.get(Integer.class); @@ -276,24 +278,24 @@ public class WeakKeySetTest { Object source2 = new Object(); Object source3 = new Object(); - set.add(key1, state1, source1); + set.add(key1, bindingData1, source1); assertInSet(set, key1, 1, source1); - set.add(key2, state2, source2); + set.add(key2, bindingData2, source2); assertInSet(set, key1, 2, source1, source2); - set.add(key3, state3, source3); + set.add(key3, bindingData3, source3); assertInSet(set, key1, 3, source1, source2, source3); - WeakReference> weakKey1Ref = new WeakReference>(key1); - WeakReference> weakKey2Ref = new WeakReference>(key2); - WeakReference> weakKey3Ref = new WeakReference>(key3); - WeakReference weakSource1Ref = new WeakReference(source1); - WeakReference weakSource2Ref = new WeakReference(source2); - WeakReference weakSource3Ref = new WeakReference(source3); + WeakReference> weakKey1Ref = new WeakReference<>(key1); + WeakReference> weakKey2Ref = new WeakReference<>(key2); + WeakReference> weakKey3Ref = new WeakReference<>(key3); + WeakReference weakSource1Ref = new WeakReference<>(source1); + WeakReference weakSource2Ref = new WeakReference<>(source2); + WeakReference weakSource3Ref = new WeakReference<>(source3); Key key = key1 = key2 = key3 = Key.get(Integer.class); - state1 = null; + bindingData1 = null; awaitFullGc(); assertSourceNotInSet(set, key, source1); @@ -304,7 +306,7 @@ public class WeakKeySetTest { // GC'd. awaitClear(weakSource1Ref); - state2 = null; + bindingData2 = null; awaitFullGc(); assertSourceNotInSet(set, key, source2); assertInSet(set, key, 1, source3); @@ -316,7 +318,7 @@ public class WeakKeySetTest { // Key1 will be referenced as the key in the sources backingSet and won't be // GC'd. - state3 = null; + bindingData3 = null; awaitFullGc(); assertNotInSet(set, key); @@ -329,125 +331,148 @@ public class WeakKeySetTest { @Test public void testWeakKeySet_integration() { - Injector parentInjector = Guice.createInjector(new AbstractModule() { - @Override - protected void configure() { - bind(Integer.class).toInstance(4); - } - }); - assertNotBlacklisted(parentInjector, Key.get(String.class)); + Injector parentInjector = + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(Integer.class).toInstance(4); + } + }); + assertNotBanned(parentInjector, Key.get(String.class)); - Injector childInjector = parentInjector.createChildInjector(new AbstractModule() { - @Override - protected void configure() { - bind(String.class).toInstance("bar"); - } - }); - WeakReference weakRef = new WeakReference(childInjector); - assertBlacklisted(parentInjector, Key.get(String.class)); + Injector childInjector = + parentInjector.createChildInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(String.class).toInstance("bar"); + } + }); + WeakReference weakRef = new WeakReference<>(childInjector); + assertBanned(parentInjector, Key.get(String.class)); - // Clear the ref, GC, and ensure that we are no longer blacklisting. + // Clear the ref, GC, and ensure that we are no longer banning. childInjector = null; awaitClear(weakRef); - assertNotBlacklisted(parentInjector, Key.get(String.class)); + assertNotBanned(parentInjector, Key.get(String.class)); } - @Test public void testWeakKeySet_integration_multipleChildren() { - Injector parentInjector = Guice.createInjector(new AbstractModule() { - @Override - protected void configure() { - bind(Integer.class).toInstance(4); - } - }); - assertNotBlacklisted(parentInjector, Key.get(String.class)); - assertNotBlacklisted(parentInjector, Key.get(Long.class)); + Injector parentInjector = + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(Integer.class).toInstance(4); + } + }); + assertNotBanned(parentInjector, Key.get(String.class)); + assertNotBanned(parentInjector, Key.get(Long.class)); - Injector childInjector1 = parentInjector.createChildInjector(new AbstractModule() { - @Override - protected void configure() { - bind(String.class).toInstance("foo"); - } - }); - WeakReference weakRef1 = new WeakReference(childInjector1); - assertBlacklisted(parentInjector, Key.get(String.class)); - assertNotBlacklisted(parentInjector, Key.get(Long.class)); + Injector childInjector1 = + parentInjector.createChildInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(String.class).toInstance("foo"); + } + }); + WeakReference weakRef1 = new WeakReference<>(childInjector1); + assertBanned(parentInjector, Key.get(String.class)); + assertNotBanned(parentInjector, Key.get(Long.class)); - Injector childInjector2 = parentInjector.createChildInjector(new AbstractModule() { - @Override - protected void configure() { - bind(Long.class).toInstance(6L); - } - }); - WeakReference weakRef2 = new WeakReference(childInjector2); - assertBlacklisted(parentInjector, Key.get(String.class)); - assertBlacklisted(parentInjector, Key.get(Long.class)); + Injector childInjector2 = + parentInjector.createChildInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(Long.class).toInstance(6L); + } + }); + WeakReference weakRef2 = new WeakReference<>(childInjector2); + assertBanned(parentInjector, Key.get(String.class)); + assertBanned(parentInjector, Key.get(Long.class)); - // Clear ref1, GC, and ensure that we still blacklist. + // Clear ref1, GC, and ensure that we still ban. childInjector1 = null; awaitClear(weakRef1); - assertNotBlacklisted(parentInjector, Key.get(String.class)); - assertBlacklisted(parentInjector, Key.get(Long.class)); + assertNotBanned(parentInjector, Key.get(String.class)); + assertBanned(parentInjector, Key.get(Long.class)); - // Clear the ref, GC, and ensure that we are no longer blacklisting. + // Clear the ref, GC, and ensure that we are no longer banning. childInjector2 = null; awaitClear(weakRef2); - assertNotBlacklisted(parentInjector, Key.get(String.class)); - assertNotBlacklisted(parentInjector, Key.get(Long.class)); + assertNotBanned(parentInjector, Key.get(String.class)); + assertNotBanned(parentInjector, Key.get(Long.class)); } @Test public void testWeakKeySet_integration_multipleChildren_overlappingKeys() { - Injector parentInjector = Guice.createInjector(new AbstractModule() { - @Override - protected void configure() { - bind(Integer.class).toInstance(4); - } - }); - assertNotBlacklisted(parentInjector, Key.get(String.class)); + Injector parentInjector = + Guice.createInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(Integer.class).toInstance(4); + } + }); + assertNotBanned(parentInjector, Key.get(String.class)); - Injector childInjector1 = parentInjector.createChildInjector(new AbstractModule() { - @Override - protected void configure() { - bind(String.class).toInstance("foo"); - } - }); - WeakReference weakRef1 = new WeakReference(childInjector1); - assertBlacklisted(parentInjector, Key.get(String.class)); + Injector childInjector1 = + parentInjector.createChildInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(String.class).toInstance("foo"); + } + }); + WeakReference weakRef1 = new WeakReference<>(childInjector1); + assertBanned(parentInjector, Key.get(String.class)); - Injector childInjector2 = parentInjector.createChildInjector(new AbstractModule() { - @Override - protected void configure() { - bind(String.class).toInstance("bar"); - } - }); - WeakReference weakRef2 = new WeakReference(childInjector2); - assertBlacklisted(parentInjector, Key.get(String.class)); + Injector childInjector2 = + parentInjector.createChildInjector( + new AbstractModule() { + @Override + protected void configure() { + bind(String.class).toInstance("bar"); + } + }); + WeakReference weakRef2 = new WeakReference<>(childInjector2); + assertBanned(parentInjector, Key.get(String.class)); - // Clear ref1, GC, and ensure that we still blacklist. + // Clear ref1, GC, and ensure that we still ban. childInjector1 = null; awaitClear(weakRef1); - assertBlacklisted(parentInjector, Key.get(String.class)); + assertBanned(parentInjector, Key.get(String.class)); - // Clear the ref, GC, and ensure that we are no longer blacklisting. + // Clear the ref, GC, and ensure that we are no longer banning. childInjector2 = null; awaitClear(weakRef2); - assertNotBlacklisted(parentInjector, Key.get(String.class)); + assertNotBanned(parentInjector, Key.get(String.class)); } - private static class TestState implements State { - public State parent() { - return new TestState(); + private static class TestInjectorBindingData extends InjectorBindingData { + TestInjectorBindingData() { + super(Optional.empty()); } + @Override + public Optional parent() { + return Optional.of(new TestInjectorBindingData()); + } + + @Override public BindingImpl getExplicitBinding(Key key) { return null; } + @Override public Map, Binding> getExplicitBindingsThisLevel() { throw new UnsupportedOperationException(); } + @Override public void putBinding(Key key, BindingImpl binding) { throw new UnsupportedOperationException(); } @@ -492,66 +517,63 @@ public class WeakKeySetTest { throw new UnsupportedOperationException(); } + @Override public ScopeBinding getScopeBinding(Class scopingAnnotation) { return null; } + @Override public void putScopeBinding(Class annotationType, ScopeBinding scope) { throw new UnsupportedOperationException(); } + @Override public void addConverter(TypeConverterBinding typeConverterBinding) { throw new UnsupportedOperationException(); } - public TypeConverterBinding getConverter(String stringValue, TypeLiteral type, Errors errors, - Object source) { + @Override + public TypeConverterBinding getConverter( + String stringValue, TypeLiteral type, Errors errors, Object source) { throw new UnsupportedOperationException(); } + @Override public Iterable getConvertersThisLevel() { return ImmutableSet.of(); } + @Override public void addTypeListener(TypeListenerBinding typeListenerBinding) { throw new UnsupportedOperationException(); } - public List getTypeListenerBindings() { + @Override + public ImmutableList getTypeListenerBindings() { return ImmutableList.of(); } + @Override public void addProvisionListener(ProvisionListenerBinding provisionListenerBinding) { throw new UnsupportedOperationException(); } - public List getProvisionListenerBindings() { + @Override + public ImmutableList getProvisionListenerBindings() { return ImmutableList.of(); } + @Override public void addScanner(ModuleAnnotatedMethodScannerBinding scanner) { throw new UnsupportedOperationException(); } - public List getScannerBindings() { + @Override + public ImmutableList getScannerBindings() { return ImmutableList.of(); } - public void blacklist(Key key, State state, Object source) { - } - - public boolean isBlacklisted(Key key) { - return true; - } - - public Set getSourcesForBlacklistedKey(Key key) { - throw new UnsupportedOperationException(); - } - - public Object lock() { - throw new UnsupportedOperationException(); - } - + @Override public Map, Scope> getScopes() { return ImmutableMap.of(); } @@ -562,17 +584,17 @@ public class WeakKeySetTest { } @Override - public List getTypeListenerBindingsThisLevel() { + public ImmutableList getTypeListenerBindingsThisLevel() { throw new UnsupportedOperationException(); } @Override - public List getProvisionListenerBindingsThisLevel() { + public ImmutableList getProvisionListenerBindingsThisLevel() { throw new UnsupportedOperationException(); } @Override - public List getScannerBindingsThisLevel() { + public ImmutableList getScannerBindingsThisLevel() { throw new UnsupportedOperationException(); } } diff --git a/src/test/java/com/google/inject/internal/WeakKeySetUtils.java b/src/test/java/com/google/inject/internal/WeakKeySetUtils.java index 465a2c6..381d694 100644 --- a/src/test/java/com/google/inject/internal/WeakKeySetUtils.java +++ b/src/test/java/com/google/inject/internal/WeakKeySetUtils.java @@ -1,14 +1,13 @@ package com.google.inject.internal; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import com.google.inject.Injector; import com.google.inject.Key; - import java.util.Set; /** @@ -16,15 +15,14 @@ import java.util.Set; */ public final class WeakKeySetUtils { - private WeakKeySetUtils() { + private WeakKeySetUtils() {} + + public static void assertBanned(Injector injector, Key key) { + assertBannedState(injector, key, true); } - public static void assertBlacklisted(Injector injector, Key key) { - assertBlacklistState(injector, key, true); - } - - public static void assertNotBlacklisted(Injector injector, Key key) { - assertBlacklistState(injector, key, false); + public static void assertNotBanned(Injector injector, Key key) { + assertBannedState(injector, key, false); } public static void assertNotInSet(WeakKeySet set, Key key) { @@ -39,8 +37,8 @@ public final class WeakKeySetUtils { assertNull(set.getSources(Key.get(Integer.class))); } - public static void assertInSet(WeakKeySet set, Key key, int expectedSources, - Object... sources) { + public static void assertInSet( + WeakKeySet set, Key key, int expectedSources, Object... sources) { assertTrue(set.contains(key)); assertEquals(expectedSources, set.getSources(key).size()); for (Object source : sources) { @@ -63,17 +61,17 @@ public final class WeakKeySetUtils { assertFalse(sources.contains(source)); } - private static void assertBlacklistState(Injector injector, Key key, boolean isBlacklisted) { - // if we're expecting it to not be blacklisted, loop around and wait for threads to run. - if (!isBlacklisted) { + private static void assertBannedState(Injector injector, Key key, boolean isBanned) { + // if we're expecting it to not be banned, loop around and wait for threads to run. + if (!isBanned) { for (int i = 0; i < 10; i++) { - if (!((InjectorImpl) injector).state.isBlacklisted(key)) { + if (!((InjectorImpl) injector).getJitBindingData().isBannedKey(key)) { break; } sleep(); } } - assertEquals(isBlacklisted, ((InjectorImpl) injector).state.isBlacklisted(key)); + assertEquals(isBanned, ((InjectorImpl) injector).getJitBindingData().isBannedKey(key)); } private static void sleep() { @@ -82,6 +80,5 @@ public final class WeakKeySetUtils { } catch (InterruptedException e) { throw new RuntimeException(e); } - Thread.yield(); } } diff --git a/src/test/java/com/google/inject/name/NamesTest.java b/src/test/java/com/google/inject/name/NamesTest.java index 947e237..e404641 100644 --- a/src/test/java/com/google/inject/name/NamesTest.java +++ b/src/test/java/com/google/inject/name/NamesTest.java @@ -10,7 +10,7 @@ import com.google.inject.Guice; import com.google.inject.Injector; import com.google.inject.Key; -import org.junit.BeforeClass; +import org.junit.Before; import org.junit.Test; import java.util.Map; import java.util.Properties; @@ -19,11 +19,11 @@ public class NamesTest { @Named("foo") private String foo; - private static Named namedFoo; + private Named namedFoo; - @BeforeClass - public static void setUp() throws Exception { - namedFoo = NamesTest.class.getDeclaredField("foo").getAnnotation(Named.class); + @Before + public void setUp() throws Exception { + namedFoo = getClass().getDeclaredField("foo").getAnnotation(Named.class); } @Test diff --git a/src/test/java/com/google/inject/spi/ElementSourceTest.java b/src/test/java/com/google/inject/spi/ElementSourceTest.java index 550ac1b..a734567 100644 --- a/src/test/java/com/google/inject/spi/ElementSourceTest.java +++ b/src/test/java/com/google/inject/spi/ElementSourceTest.java @@ -1,49 +1,30 @@ package com.google.inject.spi; -import static com.google.inject.internal.InternalFlags.getIncludeStackTraceOption; import static java.lang.annotation.RetentionPolicy.RUNTIME; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; import com.google.inject.AbstractModule; import com.google.inject.Binder; import com.google.inject.Binding; import com.google.inject.BindingAnnotation; import com.google.inject.Module; - -import org.junit.Test; import java.lang.annotation.Annotation; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.Target; import java.util.List; +import junit.framework.TestCase; -/** - * Tests for {@link ElementSource}. - */ -public class ElementSourceTest { +/** Tests for {@link ElementSource}. */ +public class ElementSourceTest extends TestCase { private static final StackTraceElement BINDER_INSTALL = - new StackTraceElement("com.google.inject.spi.Elements$RecordingBinder", "install", - "Unknown Source", 234 /* line number*/); + new StackTraceElement( + "com.google.inject.spi.Elements$RecordingBinder", + "install", + "Unknown Source", + 234 /* line number*/); - @Test - public void testCallStackSize() { - ModuleSource moduleSource = createModuleSource(); - StackTraceElement[] bindingCallStack = new StackTraceElement[3]; - bindingCallStack[0] = new StackTraceElement( - "com.google.inject.spi.Elements$RecordingBinder", "bind", "Unknown Source", 200); - bindingCallStack[1] = new StackTraceElement( - "com.google.inject.spi.Elements$RecordingBinder", "bind", "Unknown Source", 100); - bindingCallStack[2] = new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$C", "configure", "Unknown Source", 100); - ElementSource elementSource = new ElementSource( - null /* No original element source */, "" /* Don't care */, moduleSource, bindingCallStack); - assertEquals(10 /* call stack size */, elementSource.getStackTrace().length); - } - - @Test - public void testGetCallStack_IntegrationTest() { + public void testGetCallStack_IntegrationTest() throws Exception { List elements = Elements.getElements(new A()); for (Element element : elements) { if (element instanceof Binding) { @@ -59,70 +40,7 @@ public class ElementSourceTest { assertEquals("com.google.inject.spi.ElementSourceTest$B", moduleClassNames.get(1)); // Module A assertEquals("com.google.inject.spi.ElementSourceTest$A", moduleClassNames.get(2)); - StackTraceElement[] callStack = elementSource.getStackTrace(); - switch (getIncludeStackTraceOption()) { - case OFF: - // Check declaring source - StackTraceElement stackTraceElement = - (StackTraceElement) elementSource.getDeclaringSource(); - assertEquals(new StackTraceElement( - "com.google.inject.spi.ElementSourceTest$C", "configure", null, -1), - stackTraceElement); - // Check call stack - assertEquals(0, callStack.length); - return; - case ONLY_FOR_DECLARING_SOURCE: - // Check call stack - assertEquals(0, callStack.length); - return; - case COMPLETE: - // Check call stack - int skippedCallStackSize = new Throwable().getStackTrace().length - 1; - assertEquals(skippedCallStackSize + 15, elementSource.getStackTrace().length); - assertEquals("com.google.inject.spi.Elements$RecordingBinder", - callStack[0].getClassName()); - assertEquals("com.google.inject.spi.Elements$RecordingBinder", - callStack[1].getClassName()); - assertEquals("com.google.inject.AbstractModule", - callStack[2].getClassName()); - // Module C - assertEquals("com.google.inject.spi.ElementSourceTest$C", - callStack[3].getClassName()); - assertEquals("configure", - callStack[3].getMethodName()); - assertEquals("Unknown Source", - callStack[3].getFileName()); - assertEquals("com.google.inject.AbstractModule", - callStack[4].getClassName()); - assertEquals("com.google.inject.spi.Elements$RecordingBinder", - callStack[5].getClassName()); - // Module B - assertEquals("com.google.inject.spi.ElementSourceTest$B", - callStack[6].getClassName()); - assertEquals("com.google.inject.spi.Elements$RecordingBinder", - callStack[7].getClassName()); - // Module A - assertEquals("com.google.inject.AbstractModule", - callStack[8].getClassName()); - assertEquals("com.google.inject.spi.ElementSourceTest$A", - callStack[9].getClassName()); - assertEquals("com.google.inject.AbstractModule", - callStack[10].getClassName()); - assertEquals("com.google.inject.spi.Elements$RecordingBinder", - callStack[11].getClassName()); - assertEquals("com.google.inject.spi.Elements", - callStack[12].getClassName()); - assertEquals("com.google.inject.spi.Elements", - callStack[13].getClassName()); - assertEquals("com.google.inject.spi.ElementSourceTest", - callStack[14].getClassName()); - // Check modules index - List indexes = elementSource.getModuleConfigurePositionsInStackTrace(); - assertEquals((int) indexes.get(0), 4); - assertEquals((int) indexes.get(1), 6); - assertEquals((int) indexes.get(2), 10); - return; - } + return; } } } @@ -131,29 +49,11 @@ public class ElementSourceTest { private ModuleSource createModuleSource() { // First module - StackTraceElement[] partialCallStack = new StackTraceElement[1]; - partialCallStack[0] = BINDER_INSTALL; - ModuleSource moduleSource = new ModuleSource(new A(), partialCallStack); + ModuleSource moduleSource = new ModuleSource(A.class, /* permitMap = */ null); // Second module - partialCallStack = new StackTraceElement[2]; - partialCallStack[0] = BINDER_INSTALL; - partialCallStack[1] = new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$A", "configure", "Unknown Source", 100); - moduleSource = moduleSource.createChild(new B(), partialCallStack); + moduleSource = moduleSource.createChild(B.class); // Third module - partialCallStack = new StackTraceElement[4]; - partialCallStack[0] = BINDER_INSTALL; - partialCallStack[1] = new StackTraceElement("class1", "method1", "Class1.java", 1); - partialCallStack[2] = new StackTraceElement("class2", "method2", "Class2.java", 2); - partialCallStack[3] = new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$B", "configure", "Unknown Source", 200); - return moduleSource.createChild(new C(), partialCallStack); - } - - @Retention(RUNTIME) - @Target({ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD}) - @BindingAnnotation - @interface SampleAnnotation { + return moduleSource.createChild(C.class); } private static class A extends AbstractModule { @@ -170,6 +70,11 @@ public class ElementSourceTest { } } + @Retention(RUNTIME) + @Target({ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD}) + @BindingAnnotation + @interface SampleAnnotation {} + private static class C extends AbstractModule { @Override public void configure() { diff --git a/src/test/java/com/google/inject/spi/ElementsTest.java b/src/test/java/com/google/inject/spi/ElementsTest.java index 750f075..407c8a0 100644 --- a/src/test/java/com/google/inject/spi/ElementsTest.java +++ b/src/test/java/com/google/inject/spi/ElementsTest.java @@ -3,7 +3,6 @@ package com.google.inject.spi; import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.inject.Asserts.assertContains; import static com.google.inject.Asserts.getDeclaringSourcePart; -import static com.google.inject.Asserts.isIncludeStackTraceComplete; import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -1342,11 +1341,6 @@ public class ElementsTest { if (!(element instanceof Message)) { ElementSource source = (ElementSource) element.getSource(); assertFalse(source.getModuleClassNames().isEmpty()); - if (isIncludeStackTraceComplete()) { - assertTrue(source.getStackTrace().length > 0); - } else { - assertEquals(0, source.getStackTrace().length); - } } if (!(visitor instanceof ExternalFailureVisitor)) { assertContains(element.getSource().toString(), getDeclaringSourcePart(ElementsTest.class)); diff --git a/src/test/java/com/google/inject/spi/ModuleSourceTest.java b/src/test/java/com/google/inject/spi/ModuleSourceTest.java index a4e5bf1..5494630 100644 --- a/src/test/java/com/google/inject/spi/ModuleSourceTest.java +++ b/src/test/java/com/google/inject/spi/ModuleSourceTest.java @@ -1,27 +1,25 @@ package com.google.inject.spi; -import static org.junit.Assert.assertEquals; import com.google.inject.AbstractModule; import com.google.inject.Binder; import com.google.inject.Module; -import org.junit.Test; +import junit.framework.TestCase; -/** - * Tests for {@link ModuleSource}. - */ -public class ModuleSourceTest { +/** Tests for {@link ModuleSource}. */ +public class ModuleSourceTest extends TestCase { private static final StackTraceElement BINDER_INSTALL = - new StackTraceElement("com.google.inject.spi.Elements$RecordingBinder", "install", - "Unknown Source", 235 /* line number*/); + new StackTraceElement( + "com.google.inject.spi.Elements$RecordingBinder", + "install", + "Unknown Source", + 235 /* line number*/); - @Test public void testOneModule() { ModuleSource moduleSource = createWithSizeOne(); checkSizeOne(moduleSource); } - @Test public void testTwoModules() { ModuleSource moduleSource = createWithSizeTwo(); checkSizeTwo(moduleSource); @@ -29,7 +27,6 @@ public class ModuleSourceTest { checkSizeOne(moduleSource); } - @Test public void testThreeModules() { ModuleSource moduleSource = createWithSizeThree(); checkSizeThree(moduleSource); @@ -41,70 +38,28 @@ public class ModuleSourceTest { private void checkSizeOne(ModuleSource moduleSource) { assertEquals(1, moduleSource.size()); - assertEquals(1, moduleSource.getStackTraceSize()); - // Check call stack - StackTraceElement[] callStack = moduleSource.getStackTrace(); - assertEquals(BINDER_INSTALL, callStack[0]); } private void checkSizeTwo(ModuleSource moduleSource) { assertEquals(2, moduleSource.size()); - assertEquals(3, moduleSource.getStackTraceSize()); - // Check call stack - StackTraceElement[] callStack = moduleSource.getStackTrace(); - assertEquals(BINDER_INSTALL, callStack[0]); - assertEquals( - new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$A", "configure", "Unknown Source", 100), - callStack[1]); - assertEquals(BINDER_INSTALL, callStack[2]); } private void checkSizeThree(ModuleSource moduleSource) { assertEquals(3, moduleSource.size()); - assertEquals(7, moduleSource.getStackTraceSize()); - // Check call stack - StackTraceElement[] callStack = moduleSource.getStackTrace(); - assertEquals(BINDER_INSTALL, callStack[0]); - assertEquals(new StackTraceElement("class1", "method1", "Unknown Source", 1), callStack[1]); - assertEquals(new StackTraceElement("class2", "method2", "Unknown Source", 2), callStack[2]); - assertEquals( - new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$B", "configure", "Unknown Source", 200), - callStack[3]); - assertEquals(BINDER_INSTALL, callStack[4]); - - assertEquals( - new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$A", "configure", "Unknown Source", 100), - callStack[5]); - assertEquals(BINDER_INSTALL, callStack[6]); } private ModuleSource createWithSizeOne() { - StackTraceElement[] partialCallStack = new StackTraceElement[1]; - partialCallStack[0] = BINDER_INSTALL; - return new ModuleSource(new A(), partialCallStack); + return new ModuleSource(A.class, null); } private ModuleSource createWithSizeTwo() { ModuleSource moduleSource = createWithSizeOne(); - StackTraceElement[] partialCallStack = new StackTraceElement[2]; - partialCallStack[0] = BINDER_INSTALL; - partialCallStack[1] = new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$A", "configure", "moduleSourceTest.java", 100); - return moduleSource.createChild(new B(), partialCallStack); + return moduleSource.createChild(B.class); } private ModuleSource createWithSizeThree() { ModuleSource moduleSource = createWithSizeTwo(); - StackTraceElement[] partialCallStack = new StackTraceElement[4]; - partialCallStack[0] = BINDER_INSTALL; - partialCallStack[1] = new StackTraceElement("class1", "method1", "Class1.java", 1); - partialCallStack[2] = new StackTraceElement("class2", "method2", "Class2.java", 2); - partialCallStack[3] = new StackTraceElement( - "com.google.inject.spi.moduleSourceTest$B", "configure", "moduleSourceTest.java", 200); - return moduleSource.createChild(new C(), partialCallStack); + return moduleSource.createChild(C.class); } private static class A extends AbstractModule { @@ -122,6 +77,7 @@ public class ModuleSourceTest { } private static class C extends AbstractModule { + @Override public void configure() { } diff --git a/src/test/java/com/google/inject/spi/SpiBindingsTest.java b/src/test/java/com/google/inject/spi/SpiBindingsTest.java index 3ef3a47..39f1556 100644 --- a/src/test/java/com/google/inject/spi/SpiBindingsTest.java +++ b/src/test/java/com/google/inject/spi/SpiBindingsTest.java @@ -2,7 +2,6 @@ package com.google.inject.spi; import static com.google.inject.Asserts.assertContains; import static com.google.inject.Asserts.getDeclaringSourcePart; -import static com.google.inject.Asserts.isIncludeStackTraceComplete; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; @@ -431,11 +430,6 @@ public class SpiBindingsTest { assertContains(binding.getSource().toString(), getDeclaringSourcePart(getClass())); ElementSource source = (ElementSource) binding.getSource(); assertFalse(source.getModuleClassNames().isEmpty()); - if (isIncludeStackTraceComplete()) { - assertTrue(source.getStackTrace().length > 0); - } else { - assertEquals(0, source.getStackTrace().length); - } } public void checkInjector(Module module, ElementVisitor... visitors) {