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

Dart: do not derive from classes annotated as 'visibleForTesting' #1642

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pwrobeldev
Copy link
Contributor

----- Motivation -----
In order to allow users to mock static functions in classes
the 'prototype' approach was implemented in the past. It uses
prototype class to redirect static function calls.

A class with '$Impl' suffix is created as a prototype. It is
annotated as 'visibleForTesting' and exported from the library.

The user may provide its own implementation in tests and this
way redirect the calls to the mock (after setting it as prototype).

Sadly, when a class is derived from the class, which has static
methods, then its '$Impl' derives from the '$Impl' of base class.
This leads to linter warnings: 'invalid_use_of_visible_for_testing_member'.

----- Implemented solution -----
This change introduces a new type called '$HiddenImpl', which is
defined only when the class is open and has static functions.
The '$HiddenImpl' is not exported and therefore, the user cannot use it
directly. Still, the user may use the old approach to redirect the calls
to mock.

The '$HiddenImpl' is derived from an ordinary '$Impl' and its usage does
not generate linter warnings when the '$Impl' of derived class uses it,
because '$HiddenImpl' is not annotated as 'visibleForTesting'.

The '$HiddenImpl' is used only to provide implementation of methods from
the base class.

This change introduces a new LIME file to smoke
tests, which causes generation of the code for
base and derived classes, which have static methods.

The classes are generated only for Dart and the intention
is to show how 'visibleForTesting' annotation is used.

Sadly, the base type annotated as visible for testing is
used as a base class of the derived impl type. It causes
linter warnings.

Signed-off-by: Patryk Wrobel <[email protected]>
This change introduces a new LIME file to functional
tests, which causes generation of the code for
base and derived classes, which have static methods.

This will be used as a reference to confirm
that the code builds and runs after the
fix is implemented.

Signed-off-by: Patryk Wrobel <[email protected]>
In order to allow users to mock static functions in classes
the 'prototype' approach was implemented in the past. It uses
prototype class to redirect static function calls.

A class with '$Impl' suffix is created as a prototype. It is
annotated as 'visibleForTesting' and exported from the library.

The user may provide its own implementation in tests and this
way redirect the calls to the mock (after setting it as prototype).

Sadly, when a class is derived from the class, which has static
methods, then its '$Impl' derives from the '$Impl' of base class.
This leads to linter warnings: 'invalid_use_of_visible_for_testing_member'.

This change introduces a new type called '$HiddenImpl', which is
defined only when the class is open and has static functions.
The '$HiddenImpl' is not exported and therefore, the user cannot use it
directly. Still, the user may use the old approach to redirect the calls
to mock.

The '$HiddenImpl' is derived from an ordinary '$Impl' and its usage does
not generate linter warnings when the '$Impl' of derived class uses it.

The '$HiddenImpl' is used only to provide implementation of methods from
the base class.

Signed-off-by: Patryk Wrobel <[email protected]>
This commit adjusts the expected output of smoke
tests for generated classes, which reflects the
change in mustache templates.

'$HiddenImpl' classes are generated when needed.
In places where the class was  derived from 'visibleForTesting'
type now '$HiddenImpl' is used.

Signed-off-by: Patryk Wrobel <[email protected]>
This commit implements a new test case, which
verifies that mocking of static methods still
can be achieved in the same way that it was
in the past to ensure, that the users, who use
mocks do not experience any problems.

Signed-off-by: Patryk Wrobel <[email protected]>
@pwrobeldev
Copy link
Contributor Author

A note for reviewers:

The initial design of mock-ability of static functions in Dart can be found here:

@pwrobeldev pwrobeldev marked this pull request as ready for review December 19, 2024 14:23
Signed-off-by: Patryk Wrobel <[email protected]>
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.

1 participant