gaul / modernizer-maven-plugin

Detect uses of legacy Java APIs
Apache License 2.0
368 stars 53 forks source link

@SuppressModernizer not applicable to type use #155

Open delanym opened 1 year ago

delanym commented 1 year ago

I'm unable to suppress the StringBuffer to StringBuilder recommentation:

StringBuffer sb = new @SuppressModernizer StringBuffer();

If I say

@SuppressModernizer
StringBuffer sb = new StringBuffer();

I get

'@SuppressModernizer' not applicable to local variable

delanym commented 1 year ago

I could maybe use the exclusions tag, but what the heck is a javap format: java/lang/String.getBytes:(Ljava/lang/String;)[B supposed to mean I dont know. Can you include a couple of examples? Presumably most people using this plugin have no knowledge of bytecode internals.

gaul commented 1 year ago

@stevegutz could you add support for @SupressModernizer on local variables? I believe that Java 8 and later allow this. Adding support for constructors was easy via #126.

@delanym this mangled name is unusual but unambiguous. If you run your class file through javap -c you should be able to grep for the method name and see the type mangling. If you have any better suggestions please open an issue so we can improve this.

stevie400 commented 1 year ago

Hi, my org is using error-prone instead of modernizer so I'm not going to be contributing any more.

rovarga commented 1 year ago

@gaul Perhaps the first step is to add ElementType.TYPE_USE, which should be easy if annotations were bumped to 1.8 or if it were a multi-release jar. The second part is dealing with the annotation at detection point, I think: a TYPE_USE occurrence needs to be propagated to the corresponding variable allocation, which again would require JDK8+ (9+ in case of multi-release jar) as the runtime. That would satisfy everyone at Java 11+ (mulit-release, which works for Java 9+). Java 8 users would be left off no worse than they are now.

gaul commented 1 year ago

I'm not too sure what the current minimum version is but JDK 6 used to work. I am happy to declare Java 8 the new minimum so Modernizer can access ElementType.TYPE_USE. Can you propose a PR for this?

gaul commented 1 year ago

I would like the next 2.6.0 release to include this feature. @delanym @rovarga could you look into this?

rovarga commented 1 year ago

Yes, PR coming out in a few minutes.

rovarga commented 1 year ago

171 , but it also touches definitions -- sun.misc.BASE64Decoder/Encoder no longer exist.

delanym commented 1 year ago

Missed the release :(

gaul commented 1 year ago

Just to be clear there is no fix provided. There is an open linked PR with no explanation for what it does.

gaul commented 4 months ago

I still don't understand if this is possible to fix. https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.4.2 says:

An annotation on a local variable declaration is never retained in the binary representation.