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

[Feature request] Automatically disable checks based on target Java version #4208

Open
Marcono1234 opened this issue Dec 2, 2023 · 3 comments

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Dec 2, 2023

Some checks are only relevant for a certain Java version:

Check Minimum Java version
JavaUtilDate 8 (unless Joda-Time is considered a good solution as well)
PreferJavaTimeOverload ? 8
UnnecessaryAnonymousClass 8
UnnecessaryFinal 8
TryWithResourcesVariable 9
Varifier 10
StatementSwitchToExpressionSwitch 14 (check already considers this)

Would it be possible that Error Prone could automatically disable such checks based on the target Java version (-source & -target / --release) used during compilation?

There would be the following advantages with this:

  • For a Maven project where the main code has a lower target version than the test code, that would allow to enable checks which are (for now) only relevant for test code, without needing two separate compiler configurations for main and test code
  • You could enable checks which you would like to enforce in the future once you increase the target version of your code; if you only add those checks with :OFF for now, then it is likely that you forget to enable them again
  • It might allow enabling by default some Error Prone checks which are currently disabled by default (possibly because they are only relevant for a certain Java version)

As side note: The Error Prone checks mentioned above are currently all disabled by default and have to be enabled manually. But as mentioned in the list above, even in that case it would be useful if they would be ignored based on the Java target version (or at least there could be an opt-in Error Prone flag which makes it behave that way).

@Stephan202
Copy link
Contributor

Hi @Marcono1234! For StatementSwitchToExpressionSwitch such skip-if-unsupported logic is already in place; did you observe that not working as expected?

Assuming that logic does work for you: the other checks could likely be tweaked accordingly.

@Marcono1234
Copy link
Contributor Author

Ah sorry you are right, for StatementSwitchToExpressionSwitch it seems to work as desired. I only had a look at some of the listed checks and noticed that they don't consider the target Java version, but then listed additional Java version dependent checks without verifying how they behave.

Assuming that logic does work for you: the other checks could likely be tweaked accordingly.

Yes that sounds like a good idea! I have extended the list above with some more checks which might be Java target version dependent.

However, would it make sense to implement a more general solution which allows checks to specify an optional minimum Java version? And then maybe directly disable the checks on startup instead of executing them all the time despite them doing nothing (but still causing some overhead I guess); or can the target / source Java version differ between the files to be compiled?

@cushon
Copy link
Collaborator

cushon commented Dec 19, 2023

However, would it make sense to implement a more general solution which allows checks to specify an optional minimum Java version? And then maybe directly disable the checks on startup instead of executing them all the time despite them doing nothing (but still causing some overhead I guess); or can the target / source Java version differ between the files to be compiled?

I think that could make sense, and the source version should always be the same for the entire compilation. I'm a little skeptical that the overhead of the approach StatementSwitchToExpressionSwitch uses is going to be noticeable in practice, so adding a separate API for checks to express a required source version doesn't feel like a high priority to me. But I'm open to being persuaded if you're seeing overhead from that in practice.

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

No branches or pull requests

3 participants