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

Add matchers for incompatible type matchers #1832

Merged
merged 1 commit into from Dec 7, 2019

Conversation

TimvdLippe
Copy link
Contributor

We discovered that users run into issues with using the wrong Mockito
matcher for arguments. Examples include any(Integer.class) instead of
anyInt() and anyInt() instead of anyFloat(). Users then run into
cryptic run-time errors that are difficult to understand.

These ErrorProne checkers make these a compile warning, to warn the user
before hand. They also provide the appropriate fixes that can be
directly applied.

We discovered that users run into issues with using the wrong Mockito
matcher for arguments. Examples include `any(Integer.class)` instead of
`anyInt()` and `anyInt()` instead of `anyFloat()`. Users then run into
cryptic run-time errors that are difficult to understand.

These ErrorProne checkers make these a compile warning, to warn the user
before hand. They also provide the appropriate fixes that can be
directly applied.
@codecov-io
Copy link

codecov-io commented Nov 27, 2019

Codecov Report

Merging #1832 into release/3.x will decrease coverage by 0.07%.
The diff coverage is 79.03%.

Impacted file tree graph

@@                Coverage Diff                @@
##             release/3.x    #1832      +/-   ##
=================================================
- Coverage          86.71%   86.64%   -0.08%     
- Complexity          2492     2507      +15     
=================================================
  Files                311      314       +3     
  Lines               6551     6613      +62     
  Branches             822      829       +7     
=================================================
+ Hits                5681     5730      +49     
- Misses               674      682       +8     
- Partials             196      201       +5
Impacted Files Coverage Δ Complexity Δ
.../bugpatterns/MockitoAnyClassWithPrimitiveType.java 100% <100%> (ø) 4 <4> (?)
.../bugpatterns/MockitoAnyIncorrectPrimitiveType.java 100% <100%> (ø) 4 <4> (?)
...ugpatterns/AbstractMockitoAnyForPrimitiveType.java 72.91% <72.91%> (ø) 7 <7> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc6eadc...4b3cedf. Read the comment docs.

@ChristianSchwarz
Copy link
Contributor

@TimvdLippe
Why not fix the implementation of any(Class<T> type) ? any(..) can check if the passed type is a primitive and then delegate to the required matcher.

Sample:

public static <T> T any(Class<T> type) {
        if (type==int.class || type==Integer.class){ 
            return anyInt()
        }

        reportMatcher(new InstanceOf.VarArgAware(type, "<any " + type.getCanonicalName() + ">"));
        return defaultValue(type);
}

@TimvdLippe
Copy link
Contributor Author

@ChristianSchwarz That would lead to a lot of confusing code. E.g. half of the matchers would be anyInt and other half would be any(Integer.class). I would also be hesitant to introducing an API that takes the primitives as classes.

For now, we can keep this checker and we can revisit your suggestion in the future. Do you mind filing a bug?

@TimvdLippe
Copy link
Contributor Author

I am merging this for now, as it upstreams a checker that prevents real issues on runtime. We can change the Mockito API, but I would rather prevent users from running into runtime exceptions when we can.

@TimvdLippe TimvdLippe merged commit dd68237 into release/3.x Dec 7, 2019
@TimvdLippe TimvdLippe deleted the error-prone-matcher-checkers branch December 7, 2019 17:56
epeee pushed a commit to epeee/mockito that referenced this pull request Jun 22, 2020
We discovered that users run into issues with using the wrong Mockito
matcher for arguments. Examples include `any(Integer.class)` instead of
`anyInt()` and `anyInt()` instead of `anyFloat()`. Users then run into
cryptic run-time errors that are difficult to understand.

These ErrorProne checkers make these a compile warning, to warn the user
before hand. They also provide the appropriate fixes that can be
directly applied.
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 this pull request may close these issues.

None yet

3 participants