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

PreferJavaTimeOverload fails on AssertJ extensions which handle time #1435

Closed
carterkozak opened this issue Dec 5, 2019 · 3 comments
Closed
Assignees

Comments

@carterkozak
Copy link
Contributor

Description of the problem / feature request:

PreferJavaTimeOverload fails eroniously on assertj assertion classes which have time overloads, or extend org.assertj.core.api.Assertions.

Feature requests: what underlying problem are you trying to solve with this feature?

Consider allowing methods named assertThat which return subtypes of org.assertj.core.api.Assert to pass validation rather than whitelisting only org.assertj.core.api.Assertions.assertThat

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Easiest way to reproduce this is by subclassing Assertions (nonfinal utility class, obviously it's bad practice to extend static utility classes in modern java, but this occurs relatively frequently in the wild).

What version of Error Prone are you using?

2.3.4

Have you found anything relevant by searching the web?

no.

@Stephan202
Copy link
Contributor

Relates to #1377; sounds like a subtype-aware variation on f1df86d will fix this.

carterkozak added a commit to carterkozak/error-prone that referenced this issue Dec 5, 2019
carterkozak added a commit to carterkozak/error-prone that referenced this issue Dec 5, 2019
carterkozak added a commit to carterkozak/error-prone that referenced this issue Dec 5, 2019
@kluever kluever self-assigned this Dec 5, 2019
cgdecker pushed a commit that referenced this issue Dec 9, 2019
…hat or assumeThat.

Fixes #1437 and #1435

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=284455194
cgdecker pushed a commit that referenced this issue Dec 9, 2019
…hat or assumeThat.

Fixes #1437 and #1435

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=284455194
@php-coder
Copy link

Easiest way to reproduce this is by subclassing Assertions (nonfinal utility class, obviously it's bad practice to extend static utility classes in modern java, but this occurs relatively frequently in the wild).

In my case, a test implements WithAssertions that also fired this check but it doesn't seem a bad practice.

kmclarnon pushed a commit to HubSpot/error-prone that referenced this issue Feb 19, 2020
…hat or assumeThat.

Fixes google#1437 and google#1435

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=284455194
@kluever
Copy link
Member

kluever commented Apr 4, 2020

I think this can be closed now. Please re-open if you'd like additional opt-out heuristics

@kluever kluever closed this as completed Apr 4, 2020
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

Successfully merging a pull request may close this issue.

4 participants