You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/09/04 07:30:21 UTC

[GitHub] [lucene-solr] dweiss opened a new pull request #1828: LUCENE-9497: add google error prone checks

dweiss opened a new pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] madrob commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483663516



##########
File path: gradle/validation/error-prone.gradle
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+allprojects { prj ->
+  plugins.withType(JavaPlugin) {
+    prj.apply plugin: 'net.ltgt.errorprone'
+
+    dependencies {
+      errorprone("com.google.errorprone:error_prone_core")
+    }
+
+    tasks.withType(JavaCompile) { task ->
+      options.errorprone.errorproneArgs = [

Review comment:
       I would also like to see `options.errorprone.disableWarningsInGeneratedCode = true`

##########
File path: gradle/validation/error-prone.gradle
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+allprojects { prj ->
+  plugins.withType(JavaPlugin) {
+    prj.apply plugin: 'net.ltgt.errorprone'
+
+    dependencies {
+      errorprone("com.google.errorprone:error_prone_core")
+    }
+
+    tasks.withType(JavaCompile) { task ->
+      options.errorprone.errorproneArgs = [
+          // test
+          '-Xep:ExtendingJUnitAssert:OFF',

Review comment:
       Personal style, but I think ```options.errorprone {
   disable 'ExtendingJUnitAssert'
   }```
   is more clear than using `-Xep`?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] vthacker commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
vthacker commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483794345



##########
File path: gradle/validation/error-prone.gradle
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+allprojects { prj ->
+  plugins.withType(JavaPlugin) {
+    prj.apply plugin: 'net.ltgt.errorprone'
+
+    dependencies {
+      errorprone("com.google.errorprone:error_prone_core")
+    }
+
+    tasks.withType(JavaCompile) { task ->
+      options.errorprone.errorproneArgs = [
+          // test
+          '-Xep:ExtendingJUnitAssert:OFF',

Review comment:
       sounds good! I'll take the current branch and add two things and then commit the code
   1. `options.errorprone.disableWarningsInGeneratedCode = true`
   2. CHANGES entry 




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] vthacker commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
vthacker commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483717480



##########
File path: gradle/validation/error-prone.gradle
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+allprojects { prj ->
+  plugins.withType(JavaPlugin) {
+    prj.apply plugin: 'net.ltgt.errorprone'
+
+    dependencies {
+      errorprone("com.google.errorprone:error_prone_core")
+    }
+
+    tasks.withType(JavaCompile) { task ->
+      options.errorprone.errorproneArgs = [

Review comment:
       Let's add this ? I probably missed it in my PR




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dweiss commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483689014



##########
File path: gradle/validation/error-prone.gradle
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+allprojects { prj ->
+  plugins.withType(JavaPlugin) {
+    prj.apply plugin: 'net.ltgt.errorprone'
+
+    dependencies {
+      errorprone("com.google.errorprone:error_prone_core")
+    }
+
+    tasks.withType(JavaCompile) { task ->
+      options.errorprone.errorproneArgs = [

Review comment:
       I suggested the same to Varun.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] vthacker closed pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
vthacker closed pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] vthacker commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
vthacker commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483719049



##########
File path: lucene/core/src/java/org/apache/lucene/analysis/CharArrayMap.java
##########
@@ -523,6 +523,7 @@ public void clear() {
    * @throws NullPointerException
    *           if the given map is <code>null</code>.
    */
+  @SuppressWarnings("ReferenceEquality")

Review comment:
       This was the example you wanted to try out on how to suppress legitimate warnings of ReferenceEquality ( or any other warnings ) when we start enabling the checks ? 




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dweiss commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483793353



##########
File path: gradle/validation/error-prone.gradle
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+allprojects { prj ->
+  plugins.withType(JavaPlugin) {
+    prj.apply plugin: 'net.ltgt.errorprone'
+
+    dependencies {
+      errorprone("com.google.errorprone:error_prone_core")
+    }
+
+    tasks.withType(JavaCompile) { task ->
+      options.errorprone.errorproneArgs = [
+          // test
+          '-Xep:ExtendingJUnitAssert:OFF',

Review comment:
       Varun - feel free to take this branch (or patch) and roll it out on yours. I didn't intend it to be committed, I just wanted to show what's needed for it to compile and work.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] vthacker commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
vthacker commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483720288



##########
File path: gradle/validation/error-prone.gradle
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+allprojects { prj ->
+  plugins.withType(JavaPlugin) {
+    prj.apply plugin: 'net.ltgt.errorprone'
+
+    dependencies {
+      errorprone("com.google.errorprone:error_prone_core")
+    }
+
+    tasks.withType(JavaCompile) { task ->
+      options.errorprone.errorproneArgs = [
+          // test
+          '-Xep:ExtendingJUnitAssert:OFF',

Review comment:
       I am okay with either of the two styles. Ideally we'd want this list to get much shorter soon :) 




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dweiss commented on a change in pull request #1828: LUCENE-9497: add google error prone checks

Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1828:
URL: https://github.com/apache/lucene-solr/pull/1828#discussion_r483793090



##########
File path: lucene/core/src/java/org/apache/lucene/analysis/CharArrayMap.java
##########
@@ -523,6 +523,7 @@ public void clear() {
    * @throws NullPointerException
    *           if the given map is <code>null</code>.
    */
+  @SuppressWarnings("ReferenceEquality")

Review comment:
       Yes, exactly. I wanted the pr to include an example of how this can be done.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org