[GitHub] [lucene-solr] vthacker commented on pull request #1816: LUCENE-9497: Integerate Error Prone ( Static Analysis Tool ) during compilation
vthacker commented on pull request #1816: URL: https://github.com/apache/lucene-solr/pull/1816#issuecomment-686774814 Latest change applies errorprone to all modules ( thanks `forbidden-apis.gradle` for the example ) Also explicitly disable all warnings This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] vthacker commented on pull request #1816: LUCENE-9497: Integerate Error Prone ( Static Analysis Tool ) during compilation
vthacker commented on pull request #1816: URL: https://github.com/apache/lucene-solr/pull/1816#issuecomment-686719941 > but I'd like to see how to mark valid code with suppressions first so that we can correct the errors, exclude false-positives somehow and then enable this plugin to run by default. Instead of marking code with supressions, what if we excluded all the warning types that are there in our codebase explicitly ``` options.errorprone.errorproneArgs = ['-Xep:ClassCanBeStatic:OFF', '-Xep:InvalidInlineTag:OFF', ``` That would ensure 1. No new warnings creep in 1. We don't need to remove `-Werror` and the compile output doesn't throw a bunch of warnings that no one looks at Then in subsequent PRs we can remove one warning type and fix the code usage by either addressing them or suppressing them in code? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] vthacker commented on pull request #1816: LUCENE-9497: Integerate Error Prone ( Static Analysis Tool ) during compilation
vthacker commented on pull request #1816:
URL: https://github.com/apache/lucene-solr/pull/1816#issuecomment-68663
This is the set of warnings/errors for `solr/core`
```
~/apache-work/lucene-solr (solr_core_errorprone) $ gradlew -p solr/core/
compileJava
> Task :solr:solrj:compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
> Task :lucene:spatial-extras:compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
> Task :lucene:analysis:common:compileJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
> Task :lucene:suggest:compileJava
Note:
/Users/vthacker/apache-work/lucene-solr/lucene/suggest/src/java/org/apache/lucene/search/suggest/jaspell/JaspellLookup.java
uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
> Task :solr:core:compileJava
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java:93:
warning: [UnnecessaryParentheses] These grouping parentheses are unnecessary;
it is unlikely the code will be misinterpreted without them
if (obj.get(s) == null || (!(obj.get(s) instanceof Map))) {
^
(see https://errorprone.info/bugpattern/UnnecessaryParentheses)
Did you mean 'if (obj.get(s) == null || !(obj.get(s) instanceof Map)) {'?
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java:112:
warning: [JdkObsolete] It is very rare for LinkedList to out-perform ArrayList
or ArrayDeque. Avoid it unless you're willing to invest a lot of time into
benchmarking. Caveat: LinkedList supports null elements, but ArrayDeque does
not.
LinkedList hierarchy = new LinkedList<>();
^
(see https://errorprone.info/bugpattern/JdkObsolete)
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java:130:
warning: [UnnecessaryParentheses] These grouping parentheses are unnecessary;
it is unlikely the code will be misinterpreted without them
if (obj.get(s) == null || (!(obj.get(s) instanceof Map))) {
^
(see https://errorprone.info/bugpattern/UnnecessaryParentheses)
Did you mean 'if (obj.get(s) == null || !(obj.get(s) instanceof Map)) {'?
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/schema/FieldType.java:1265:
warning: [ReferenceEquality] Comparison using reference equality instead of
value equality
if (analyzer.getVersion() != Version.LATEST) {
^
(see https://errorprone.info/bugpattern/ReferenceEquality)
Did you mean 'if (!Objects.equals(analyzer.getVersion(), Version.LATEST))
{' or 'if (!analyzer.getVersion().equals(Version.LATEST)) {'?
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/core/PluginInfo.java:187:
warning: [MissingOverride] toMap implements method in MapSerializable;
expected @Override
public Map toMap(Map map) {
^
(see https://errorprone.info/bugpattern/MissingOverride)
Did you mean '@Override @SuppressWarnings({"unchecked", "rawtypes"})'?
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/core/SolrConfig.java:319:
warning: [ReferenceEquality] Comparison using reference equality instead of
value equality
if (version == Version.LATEST &&
!versionWarningAlreadyLogged.getAndSet(true)) {
^
(see https://errorprone.info/bugpattern/ReferenceEquality)
Did you mean 'if (Objects.equals(version, Version.LATEST) &&
!versionWarningAlreadyLogged.getAndSet(true)) {' or 'if
(version.equals(Version.LATEST) &&
!versionWarningAlreadyLogged.getAndSet(true)) {'?
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java:88:
warning: [PublicConstructorForAbstractClass] Constructors of on an abstract
class can be declared protected as there is never a need for them to be public
public RequestHandlerBase() {
^
(see
https://errorprone.info/bugpattern/PublicConstructorForAbstractClass)
Did you mean 'protected RequestHandlerBase() {'?
/Users/vthacker/apache-work/lucene-solr/solr/core/src/java/org/apache/solr/security/PermissionNameProvider.java:60:
warning: [ImmutableEnumChecker] enums should be immutable: 'Name' has field
'collName' of type 'java.util.Set', 'Set' is mutable
final Set collName;
^
(see https://errorprone.info/bugpattern/ImmutableEnumChecker)
[GitHub] [lucene-solr] vthacker commented on pull request #1816: LUCENE-9497: Integerate Error Prone ( Static Analysis Tool ) during compilation
vthacker commented on pull request #1816:
URL: https://github.com/apache/lucene-solr/pull/1816#issuecomment-686220411
Here is the complete list of warnings/errors that this tool gives us for
lucene-core
```
~/apache-work/lucene-solr (solr_core_errorprone) $ gradlew -p lucene/core/
compileJava
> Task :lucene:core:compileJava
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/Sort.java:180:
warning: [ReferenceEquality] Comparison using reference equality instead of
value equality
if (fields[i] != rewrittenSortFields[i]) {
^
(see https://errorprone.info/bugpattern/ReferenceEquality)
Did you mean 'if (!Objects.equals(fields[i], rewrittenSortFields[i])) {'
or 'if (!fields[i].equals(rewrittenSortFields[i])) {'?
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/Sort.java:185:
warning: [UnnecessaryParentheses] These grouping parentheses are unnecessary;
it is unlikely the code will be misinterpreted without them
return (changed) ? new Sort(rewrittenSortFields) : this;
^
(see https://errorprone.info/bugpattern/UnnecessaryParentheses)
Did you mean 'return changed ? new Sort(rewrittenSortFields) : this;'?
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java:690:
warning: [ModifiedButNotUsed] A collection or proto builder was created, but
its values were never accessed.
final List collectedCollectors = new ArrayList<>();
^
(see https://errorprone.info/bugpattern/ModifiedButNotUsed)
Did you mean to remove this line or to remove this line?
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/IndexSearcher.java:756:
warning: [ReferenceEquality] Comparison using reference equality instead of
value equality
for (Query rewrittenQuery = query.rewrite(reader); rewrittenQuery !=
query;
^
(see https://errorprone.info/bugpattern/ReferenceEquality)
Did you mean 'for (Query rewrittenQuery = query.rewrite(reader);
!Objects.equals(rewrittenQuery, query);' or 'for (Query rewrittenQuery =
query.rewrite(reader); !rewrittenQuery.equals(query);'?
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java:98:
warning: [PublicConstructorForAbstractClass] Constructors of on an abstract
class can be declared protected as there is never a need for them to be public
public Similarity() {}
^
(see
https://errorprone.info/bugpattern/PublicConstructorForAbstractClass)
Did you mean 'protected Similarity() {}'?
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java:111:
warning: [EscapedEntity] HTML entities in @code/@literal tags will appear
literally in the rendered javadoc.
* {@code Long.compareUnsigned(n1, n2) 0} then
^
(see https://errorprone.info/bugpattern/EscapedEntity)
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java:112:
warning: [EscapedEntity] HTML entities in @code/@literal tags will appear
literally in the rendered javadoc.
* {@code SimScorer.score(freq, n1) = SimScorer.score(freq, n2)}
^
(see https://errorprone.info/bugpattern/EscapedEntity)
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java:157:
warning: [EscapedEntity] HTML entities in @code/@literal tags will appear
literally in the rendered javadoc.
* {@code freq1 freq2}, then {@code score(freq1, norm) =
^
(see https://errorprone.info/bugpattern/EscapedEntity)
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java:157:
warning: [EscapedEntity] HTML entities in @code/@literal tags will appear
literally in the rendered javadoc.
* {@code freq1 freq2}, then {@code score(freq1, norm) =
^
(see https://errorprone.info/bugpattern/EscapedEntity)
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java:162:
warning: [EscapedEntity] HTML entities in @code/@literal tags will appear
literally in the rendered javadoc.
* {@code Long.compareUnsigned(norm1, norm2) 0} then
^
(see https://errorprone.info/bugpattern/EscapedEntity)
/Users/vthacker/apache-work/lucene-solr/lucene/core/src/java/org/apache/lucene/search/similarities/Similarity.java:163:
warning: [EscapedEntity] HTML entities in @code/@literal tags will appear
literally in the rendered javadoc.
* {@code score(freq,
[GitHub] [lucene-solr] vthacker commented on pull request #1816: LUCENE-9497: Integerate Error Prone ( Static Analysis Tool ) during compilation
vthacker commented on pull request #1816: URL: https://github.com/apache/lucene-solr/pull/1816#issuecomment-686220105 > Why is there this strange dependency? Listing it under plugins should be enough. Ah okay! Just getting starter with gradle so I'll be making some mistakes like that :) The error prone [docs](https://errorprone.info/docs/installation) point to this plugin for gradle integration. Hopefully this change is more in tune with how to add error-prone To get this plugin integrated ( without fixing all the warnings/errors ) we'd need to set this `options.errorprone.allErrorsAsWarnings = true` and remove `"-Werror",` from the `defaults-java.gradle` file This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] vthacker commented on pull request #1816: LUCENE-9497: Integerate Error Prone ( Static Analysis Tool ) during compilation
vthacker commented on pull request #1816: URL: https://github.com/apache/lucene-solr/pull/1816#issuecomment-685924654 Looks like this is duplicate of LUCENE-9411? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
