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

Treat type annotations in (generic) method and class signatures as API #372

Closed
vlsi opened this issue Mar 4, 2021 · 8 comments · Fixed by #384
Closed

Treat type annotations in (generic) method and class signatures as API #372

vlsi opened this issue Mar 4, 2021 · 8 comments · Fixed by #384
Labels
bug Something isn't working
Milestone

Comments

@vlsi
Copy link
Contributor

vlsi commented Mar 4, 2021

Is your feature request related to a problem? Please describe.

It looks like the plugin does not always detect type annotations, so it suggests api -> implementation while the proper configuration should be api.

I do not have a standalone sample yet, and it looks like this issue results in false-positives for Caclite code: apache/calcite#2362

Describe the solution you'd like

I expect that dependency-analysis should gather type annotations from all signatures (including generic signatures).

Additional context

The relevant bit of the report:

  Advice for project :example:csv
  Existing dependencies which should be modified to be as indicated:
  - implementation("org.checkerframework:checker-qual:3.10.0") (was api)

AFAIK, checker-qual is annotation-only dependency.

Sample uses for @Nullable. buildHealth suggests implementation, however, the annotations are used in the signatures of the public methods, so the dependency should be api.
https://github.com/apache/calcite/blob/0fb14d553764f2a993ec56db4a36de2713ac1206/example/csv/src/main/java/org/apache/calcite/adapter/csv/CsvTableScan.java#L89
https://github.com/apache/calcite/blob/0fb14d553764f2a993ec56db4a36de2713ac1206/example/csv/src/main/java/org/apache/calcite/adapter/csv/CsvTableFactory.java#L46

@vlsi vlsi changed the title Treat type annotations as API Treat type annotations in (generic) method and class signatures as API Mar 4, 2021
@autonomousapps
Copy link
Owner

Thanks for the report @vlsi. Can you tell me which version of the plugin you were using? Version 0.68.0 included a number of fixes relating to annotations.

If the issue persists, a minimal reproducer would help a lot.

@autonomousapps autonomousapps added bug Something isn't working question Further information is requested labels Apr 18, 2021
@vlsi
Copy link
Contributor Author

vlsi commented Apr 18, 2021

I used 0.71.0

@autonomousapps autonomousapps removed the question Further information is requested label Apr 24, 2021
@autonomousapps autonomousapps added this to the 1.0 milestone Apr 24, 2021
@autonomousapps
Copy link
Owner

Thanks. I have an existing test suite covering various annotation cases, but I do not cover this scenario (yet).

@autonomousapps
Copy link
Owner

The fix for this will be in the next release.

@vlsi
Copy link
Contributor Author

vlsi commented Apr 25, 2021

Nice, thank you.

@autonomousapps
Copy link
Owner

Just released 0.73.0 with this fix.

@vlsi
Copy link
Contributor Author

vlsi commented May 6, 2021

Nice. It seems to heal all the issues with type annotations in Calcite.

@autonomousapps
Copy link
Owner

autonomousapps commented May 6, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants