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 DoNotMockRecords to warn against mocking record data objects #4384

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ralstonba
Copy link

Overview

Enhance the Java programming language with records, which are classes that act as transparent carriers for immutable data. Records can be thought of as nominal tuples.

Records are typically used as simple wrappers of data without any methods or complex behavior requiring external dependencies or expensive IO operations. As such, they should (almost) never be mocked, and instead a real instance should be used instead.

Details

The class hierarchy has changed from

|- AbstractMockChecker
  |-- DoNotMockAutoValue
  |-- DoNotMockChecker

to

|- AbstractMockChecker
  |-- DoNotMockRecords
  |-- AbstractAnnotationMockChecker
    |-- DoNotMockAutoValue
    |-- DoNotMockChecker

This was because AbstractMockChecker assumed that only annotations would be used in determining that a class should not be mocked. The majority of AbstractAnnotationMockChecker is a straight lift and shift of the original AbstractMockChecker.

Open Questions

  • ElementKind.RECORD results in an IDE error in Intellij as it is annotated with @since 16, suggesting that the language level of the module be changed. This does not cause issues with complilation, but as I am entirely new to projects that deal with the AST I am wondering if there is a better approach here.

Copy link

google-cla bot commented May 1, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@ralstonba
Copy link
Author

Signed CLA

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

1 participant