Conversation
D8H
changed the title
D8H
marked this pull request as draft
September 15, 2026 12:39
D8H
force-pushed
the
fix-bracket-error
branch
from
September 19, 2026 15:30
6385d2c to
87df623
Compare
D8H
marked this pull request as ready for review
September 19, 2026 15:34
4ian
pushed a commit
that referenced
this pull request
Sep 21, 2026
These tests reproduce 2 issues found while reviewing #9105. They are expected to fail until the corresponding code is fixed. - "primitive" is ignored for "variableOrProperty" and "variableOrPropertyOrParameter" parameters. ExpressionValidator only checks Type::Variable, Type::ObjectVariable and Type::LegacyVariable, so the 6 built-in variable conditions and actions declaring "primitive" never raise an error on a structure or an array. - isIntoBrackets is reset to false instead of being restored in OnVisitVariableBracketAccessorNode, so a nested bracket accessor makes the following empty node fall back to the generic message instead of the bracket specific one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U3gfFre6be5CeT4rhTCeST
4ian
reviewed
Sep 21, 2026
|
|
||
| namespace gd { | ||
|
|
||
| struct TypeAndExtraInfo { |
| auto parentParameterExtraInfo = currentParameterExtraInfo; | ||
| currentParameterExtraInfo = nullptr; | ||
| currentParameterExtraInfo = ""; | ||
| isIntoBrackets = true; |
Owner
There was a problem hiding this comment.
We might need a "stack" of this for nested bracket. Check this test: de4e2f8#diff-2e73971cd51c6b4bd23506151af0c5b315d285a8e5d1ce72990fa70b2421a9e2R4695
(in practice, this is only I think to make sure the error messages are as precise as possible)
Collaborator
Author
There was a problem hiding this comment.
We can make it isDirectParentBracketAccessor and put it back to false as soon as we enter another node.
| "to access. For example: `MyVariable[0]`."), | ||
| childNameLocation); | ||
| } | ||
| } else if ((parentType == Type::Variable || |
Owner
There was a problem hiding this comment.
Should we also handle Type::VariableOrProperty and Type::VariableOrPropertyOrParameter?
See new test: de4e2f8#diff-2e73971cd51c6b4bd23506151af0c5b315d285a8e5d1ce72990fa70b2421a9e2R2480
Collaborator
Author
There was a problem hiding this comment.
There are already existing errors:
- "Properties can't have children"
- "Accessing a child variable of a property is not possible"
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Uh oh!
There was an error while loading. Please reload this page.