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 2023/01/01 19:58:52 UTC
[GitHub] [lucene] rmuir opened a new issue, #12057: Forbidden-apis "built-in" signatures don't appear to be working?
rmuir opened a new issue, #12057:
URL: https://github.com/apache/lucene/issues/12057
### Description
I was looking at new error-prone checks in #12056 and one fails on Object.finalize
Because the method is in the built-in JDK deprecated list (e.g. https://github.com/policeman-tools/forbidden-apis/blob/main/src/main/resources/de/thetaphi/forbiddenapis/signatures/jdk-deprecated-11.txt#L195), I would expect the check to fail if i override finalize.
If I give `lucene/core/src/test/org/apache/lucene/TestDemo.java` a finalizer method, nothing fails. It makes me worried the built-in signatures lists aren't being applied somehow? Maybe the gradle task matching logic in the forbidden-apis config is buggy? not sure what is going on. cc: @uschindler
### Version and environment details
_No response_
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org.apache.org
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] rmuir commented on issue #12057: ban finalizers in the build somehow (worst-case: use error-prone)
Posted by GitBox <gi...@apache.org>.
rmuir commented on issue #12057:
URL: https://github.com/apache/lucene/issues/12057#issuecomment-1368933723
that's true, it is another option. I am still investigating the ECJ one.
It is a bit sad that we are so messy about deprecations and can't simply use the compiler's support. For example there are many deprecations in `main`, which makes no sense.
As an example, all the big-endian varhandles in `BitUtil` are deprecated, but they are in use across `main`, and from what I can tell, usage is only growing. That's because java uses it internally? So what is the point of the deprecation here? It is doing no good at all and just preventing us from using our compilers properly.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] rmuir closed issue #12057: ban finalizers in the build somehow (worst-case: use error-prone)
Posted by GitBox <gi...@apache.org>.
rmuir closed issue #12057: ban finalizers in the build somehow (worst-case: use error-prone)
URL: https://github.com/apache/lucene/issues/12057
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] dweiss commented on issue #12057: ban finalizers in the build somehow (worst-case: use error-prone)
Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #12057:
URL: https://github.com/apache/lucene/issues/12057#issuecomment-1368723771
Even a simplistic regexp/substring expression added to rat checks would work here, I think? this method is fairly uniquely-named. There are better alternatives but they will add overhead (parsing classes, etc).
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] rmuir commented on issue #12057: ban finalizers in the build somehow (worst-case: use error-prone)
Posted by GitBox <gi...@apache.org>.
rmuir commented on issue #12057:
URL: https://github.com/apache/lucene/issues/12057#issuecomment-1368539424
Currently there is no good way with ECJ/javac, unless we fail on all deprecations, which is very noisy at the moment. We can probably do it better with ECJ if we enable all their deprecation options, and clean up codebase (e.g. ensure tests calling deprecated stuff are also themselves deprecated)
* org.eclipse.jdt.core.compiler.problem.deprecation
* org.eclipse.jdt.core.compiler.problem.deprecationInDeprecatedCode
* org.eclipse.jdt.core.compiler.problem.deprecationWhenOverridingDeprecatedMethod
But looking at the code, that's gonna require quite a bit of work, I just wanted to make some progress here and prevent finalizers from slipping in.
Unfortunately the error-prone checker doesn't fail on this case either yet: https://github.com/google/error-prone/pull/3652
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] rmuir commented on issue #12057: Forbidden-apis "built-in" signatures don't appear to be working?
Posted by GitBox <gi...@apache.org>.
rmuir commented on issue #12057:
URL: https://github.com/apache/lucene/issues/12057#issuecomment-1368527335
Confirmed that's the issue, if i add a `super.finalize()` call to my finalizer, then forbidden fails. I will edit the issue.
So we may need to use a different tool (javac, ecj) to ban finalizers. worst-case we just enable the error-prone check for them, but I try to avoid using error-prone if something simpler will do it.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] rmuir commented on issue #12057: Forbidden-apis "built-in" signatures don't appear to be working?
Posted by GitBox <gi...@apache.org>.
rmuir commented on issue #12057:
URL: https://github.com/apache/lucene/issues/12057#issuecomment-1368524586
Here's how to reproduce: apply this patch, then run `gradlew check -x test`. I would expect the build to fail, because we added a deprecated finalizer. Maybe forbidden doesn't fail because we don't actually call Object.finalize()? This method is a little special in that overriding it is enough to be bad. Maybe we should try to fix javac or ECJ to fail on deprecated usages instead?
```
diff --git a/lucene/core/src/test/org/apache/lucene/TestDemo.java b/lucene/core/src/test/org/apache/lucene/TestDemo.java
index 6c608e1d0b1..8bcbdc813ee 100644
--- a/lucene/core/src/test/org/apache/lucene/TestDemo.java
+++ b/lucene/core/src/test/org/apache/lucene/TestDemo.java
@@ -46,6 +46,11 @@ import org.apache.lucene.util.IOUtils;
*/
public class TestDemo extends LuceneTestCase {
+ @Override
+ protected void finalize() {
+ System.out.println("YOLO");
+ }
+
public void testDemo() throws IOException {
String longTerm =
"longtermlongtermlongtermlongtermlongtermlongtermlongtermlong"
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] uschindler commented on issue #12057: ban finalizers in the build somehow (worst-case: use error-prone)
Posted by GitBox <gi...@apache.org>.
uschindler commented on issue #12057:
URL: https://github.com/apache/lucene/issues/12057#issuecomment-1368738480
Yeah, similar like the LOG statement checks in Solr?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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