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

Default models support for Spring's Autowired #477

Merged
merged 5 commits into from
May 14, 2021
Merged

Default models support for Spring's Autowired #477

merged 5 commits into from
May 14, 2021

Conversation

lazaroclapp
Copy link
Collaborator

@lazaroclapp lazaroclapp commented May 13, 2021

This adds support for spring's @Autowired annotation as both a way to exclude fields from initialization checking (since Spring will inject them before any methods are called), and to mark methods as initializers. See Spring's documentation and the discussion in #396

This is based on the original PR #475 contributed by @chat-eau-lumi (note CLA signature in that PR). It adds fixes to test cases, particularly around importing Spring classes and the constructor usage of @Autowired, contributed by @lazaroclapp.

@lazaroclapp lazaroclapp mentioned this pull request May 13, 2021
Closed
@lazaroclapp
Copy link
Collaborator Author

@chat-eau-lumi Can you take a look and make sure the Spring code for the tests is what you had in mind? It doesn't exactly need to run (since it's not being processed by the Spring annotation processor), but it should look as close to real Spring code demonstrating both @Autowired uses as possible.

@coveralls
Copy link

coveralls commented May 13, 2021

Pull Request Test Coverage Report for Build #533

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 89.162%

Files with Coverage Reduction New Missed Lines %
src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java 3 95.45%
Totals Coverage Status
Change from base Build #532: 0.0%
Covered Lines: 3480
Relevant Lines: 3903

💛 - Coveralls

@chat-eau-lumi
Copy link

Yes, it looks good! I'm used to writing public Test() instead of public init(), but it looks like init() covers the constructor case just fine.

Copy link
Collaborator

@msridhar msridhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lazaroclapp
Copy link
Collaborator Author

Yes, it looks good! I'm used to writing public Test() instead of public init(), but it looks like init() covers the constructor case just fine.

Wait a moment. Clarification: the point of making @Autowired methods initializers is to have methods which are not constructors but are annotated as @Autowired be considered initializers from the point of view of NullAway. The class constructor (Test()) is always an initializer and always checked for which fields it does or doesn't initialize.

Am I misunderstanding what @Autowired on a method means here? When would you normally use that annotation in a method (as opposed to a field)?

@chat-eau-lumi
Copy link

Sorry, I mistook the Autowired initializers case for the Autowired contructors case for some reason! I usually see Autowired setter methods, but you're right- in general it's used for initializer methods.

@lazaroclapp
Copy link
Collaborator Author

Checking the docs, it seems that what this PR does makes sense, even if it doesn't cover all the semantics of Spring's @Autowired. Will re-run CI after rebasing and land this once all checks pass :)

@lazaroclapp lazaroclapp merged commit 067c31d into master May 14, 2021
@lazaroclapp lazaroclapp deleted the pr475 branch May 14, 2021 20:12
msridhar pushed a commit that referenced this pull request Sep 20, 2023
…annotations of Spring test (#757)

This is a small followup to #477. When you are testing Spring (Boot)
applications then you are replacing controllers that are normally
injected via `@Autowired` with stubs, mocks or spies from Mockito. This
is done with two similar annotations:

- `@MockBean`
- `@SpyBean`

So it would be helpful if these are ignored as well.
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

4 participants