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

False positives when using DateTimeFormatter fluent builder #181

Closed
stonio opened this issue Sep 6, 2021 · 9 comments
Closed

False positives when using DateTimeFormatter fluent builder #181

stonio opened this issue Sep 6, 2021 · 9 comments
Assignees

Comments

@stonio
Copy link

stonio commented Sep 6, 2021

An error is raised for Java 11 code with forbidden-apis version 3.1:

DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG).withLocale(Locale.US)

Error is:

[ERROR] Forbidden method invocation: java.time.format.DateTimeFormatter#ofLocalizedDate(java.time.format.FormatStyle) [Uses default locale]

Do you consider this case a false positive error as locale is defined after method call with fluent builder pattern?

@uschindler
Copy link
Member

Hi,
you've found a problem that goes back to the Java API and that's not easy to workaround. Indeed this is one of the places where the locale can be defined using the fluent style but there's missing a way how to define the locale upfront, because theres no way to figure out from the typing system if the DateTimeFormatter is aware of a locale or not.

I would indeeed see this as a false positive, but I have no idea how to fix this issue without additional program logic inside forbiddenapis.

Do you have an idea how to "work around" this? What would be the correct way to call this without falling back to a String pattern and still giving a locale? If there is no workaround, this is indeed a bug which may need logic in forbiddenapi's code, although I am strictly against hacking workaround logic (which isn't easy to do anyways).

@uschindler
Copy link
Member

FYI, the following quote from the Javadocs already makes clear where the problem comes from:

Note that the localized pattern is looked up lazily. This DateTimeFormatter holds the style required and the locale, looking up the pattern required on demand.

Uwe

@dweiss
Copy link
Contributor

dweiss commented Sep 6, 2021

I don't think you can reasonably fix this: imagine that fluent builder is passed as an argument to a different function which sets the locale (and other possible dynamic scenarios)... You can't determine what's going to happen to it up front. The only workaround I see is to mark this type of expression with a suppressed check in the code.

@rmuir
Copy link
Member

rmuir commented Sep 6, 2021

Maybe i am old-fashioned but i prefer functions that have inputs and return values, the problem here is just a garbage API...

@uschindler
Copy link
Member

Thanks @dweiss - that's the same conclusion, I'd close this issue as "won't fix".

It's not a bug in forbiddenapis, it's a bug in the API.

@uschindler
Copy link
Member

uschindler commented Sep 6, 2021

Anyways, I found a bug in the signaturesfile of JDK-8 (which is inherited): DateTimeFormatterBuilder#toFormatter() is missing. I will open a PR.

@uschindler
Copy link
Member

uschindler commented Sep 6, 2021

I opened #182 about the missing method. I found it by staring at the API for an hour :-)

@dweiss
Copy link
Contributor

dweiss commented Sep 6, 2021

Starring can be good, Uwe. :)
https://en.wikipedia.org/wiki/The_Starry_Night#/media/File:Van_Gogh_-_Starry_Night_-_Google_Art_Project.jpg

@uschindler
Copy link
Member

If you stare (not star) at this picture for an hour in the highest resolution a lot can happen: https://commons.wikimedia.org/wiki/File:The_Garden_of_earthly_delights.jpg

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

No branches or pull requests

4 participants