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 static interface implementation check #59

Merged
merged 3 commits into from
Jan 21, 2019

Conversation

breml
Copy link
Contributor

@breml breml commented Jun 11, 2018

With the flag staticcheck set, an additional line of code per interface is
added to the generated mock file, which allows the go compiler to statically
check if the mock implements the mocked interface.

@breml
Copy link
Contributor Author

breml commented Jun 13, 2018

@matryer rebased this PR, so it should be ready to be merged.

@matryer
Copy link
Owner

matryer commented Jun 13, 2018

@breml I'm not sure we need this, wouldn't that basically happen when you come to use the mock anyway? I.e. if you're passing it into something that accepts the original interface, it'll be checked then?

@breml
Copy link
Contributor Author

breml commented Jun 14, 2018

@matryer In principle I agree. I see two advantages of this staticcheck:

  1. The compile error points you the the file where the mock is defined, instead of the file where the mock is used.
  2. If the mock is passed to a function, which accepts a narrower interface (or even the empty interface) and then an assertion is used, with the staticcheck, the compiler will still complain, whereas without the staticcheck the runtime will panic or (hopefully) a test case will fail, because a different code branch is taken after a type switch or a type assertion. In either case the problem is only visible when executing the code, whereas with the staticcheck already the compiler (and with this my editor on save) is able to detect the problem.

For me, the staticcheck adds a second line of defense, which can be achieved with a single line in a generated file. This looks for me as well worth.

@breml
Copy link
Contributor Author

breml commented Jul 4, 2018

@matryer have you considered my comment above?

@matryer
Copy link
Owner

matryer commented Jul 24, 2018

@breml If this is worth having, then we could always include it rather than it being configuration?

@breml breml changed the title Add staticcheck flag Add static interface implementation check Aug 17, 2018
@breml
Copy link
Contributor Author

breml commented Aug 17, 2018

@matryer I updated the PR. Now there is no longer a flag, the additional static check is added to every generated mock file.

Add an additional line of code per interface to the generated mock file,
which allows the go compiler to statically check if the mock implements
the mocked interface.
@breml
Copy link
Contributor Author

breml commented Jan 17, 2019

Rebased, now TravisCI is happy.

@matryer matryer merged commit bb199b0 into matryer:master Jan 21, 2019
@breml breml mentioned this pull request Jan 22, 2019
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

2 participants