fix: creating duplicate params via renaming vars #6885
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maribethb
reviewed
Mar 8, 2023
maribethb
approved these changes
Mar 8, 2023
|
Fixed lint, and set it to error since we want to enforce it! Please tell me if you want me to revert that change. |
cpcallen
reviewed
Jun 28, 2023
| @@ -166,6 +166,9 @@ Blockly.Msg.VARIABLE_ALREADY_EXISTS = 'A variable named "%1" already exists.'; | |||
| /** @type {string} */ | |||
| /// alert - Tells the user that the name they entered is already in use for another type. | |||
| Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE = 'A variable named "%1" already exists for another type: "%2".'; | |||
| /** @type {string} */ | |||
| /// alert - Tells the user that the name they entered is already in use as a parameter to a procedure, that the variable they are renaming also exists on. Renaming would create two parameters with the same name, which is not allowed. | |||
| Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_A_PARAMETER = 'A variable named "%1" already exists as a parameter in the procedure "%2".'; | |||
There was a problem hiding this comment.
Looks like you did not npm run messages (formerly …generate:langfiles), so this message is not included in any translation (including English).
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The basics
npm run formatandnpm run lintThe details
Resolves
Fixes #3767
Proposed Changes
Adds a step to
renameVariablethat detects if a variable rename will create a parameter conflict.Detects conflicts in legacy procedure blocks by walking the workspace, and conflicts in procedure models by checking if the parameter wraps a variable model.
Alternative implementations discussed here: #3767 (comment)
Reason for Changes
Squash de buggies
Test Coverage
Added tests for
nameUsedWithConflictingParamfor both legacy procedure blocks and procedure models.Documentation
N/A
Additional Information
Best reviewed commit-wise, especially because I reorganize some code to return-early instead of deeply nesting.