Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@SuppressModernizer not applicable to type use #155

Open
delanym opened this issue Nov 28, 2022 · 11 comments
Open

@SuppressModernizer not applicable to type use #155

delanym opened this issue Nov 28, 2022 · 11 comments

Comments

@delanym
Copy link
Contributor

delanym commented Nov 28, 2022

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
Copy link
Contributor Author

delanym commented Nov 28, 2022

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
Copy link
Owner

gaul commented Dec 1, 2022

@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
Copy link
Contributor

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

@rovarga
Copy link
Contributor

rovarga commented Dec 5, 2022

@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
Copy link
Owner

gaul commented Dec 6, 2022

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
Copy link
Owner

gaul commented Feb 16, 2023

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

@rovarga
Copy link
Contributor

rovarga commented Feb 16, 2023

Yes, PR coming out in a few minutes.

@rovarga
Copy link
Contributor

rovarga commented Feb 16, 2023

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

@delanym
Copy link
Contributor Author

delanym commented Apr 1, 2023

Missed the release :(

@gaul
Copy link
Owner

gaul commented Apr 1, 2023

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

@gaul
Copy link
Owner

gaul commented Feb 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants