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

Fix "Variable is implicitly Variant" quickfix results for array declarations #5647

Merged
merged 4 commits into from Feb 10, 2021

Conversation

BZngr
Copy link
Contributor

@BZngr BZngr commented Dec 2, 2020

Closes #5646

PR modifies the DeclareAsExplicitVariant to leverage the ImplicitTypeToExplicitRefactoringAction and removes the DeclareAsExplicitVariant tests as they are now redundant to the tests for the refactoring action.

Both DeclareAsExplicitVariant and DeclareAsExplicitType quickfixes now leverage the ImplicitTypeToExplicitRefactoringAction.

Removes DeclareAsExplicitVariantQuickFixTests as redundant
Copy link
Contributor

@MDoerner MDoerner left a comment

Choose a reason for hiding this comment

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

Nice! I really like that the quickfix now uses a refactoring action.
My only real concern with the PR is that the old tests for the quickfix have been removed and instead of being extended. We also have tests for the other refactoring-based quickfixes.

Tests that the QuickFix correctly invokes the supporting RefactoringAction.  Tests for various inspection result scenarios reside in the RefactoringActionTests.
Adds guard clause to constrain refactoring action to the finite set of supported types.
@BZngr
Copy link
Contributor Author

BZngr commented Dec 4, 2020

Thanks for questioning the use of dynamic. Even though it worked as part of the origin quickfix, your comment made me consider/realize something. Now that the work is done in a refactoring action, the model parameter is not guaranteed to be coming from (and vetted by) the inspection/quickfix processing. So, removed the use of dynamic and added a guard clause to enforce explicit input expectations.

I've also added unit tests for the two quickfixes. They basically test that the quickfix can successfully invoke the refactoring action. All the other various scenarios from the original quickfix were moved to the refactoring action unit tests.

@retailcoder retailcoder merged commit 0b86dd3 into rubberduck-vba:next Feb 10, 2021
@BZngr BZngr deleted the 5646_ImplicitTypeArrayFix branch February 11, 2021 03:01
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.

Fix for "Variable is implicitly Variant" breaks array declaration
4 participants