Skip to content

fix(parser): clarify consecutive comparison diagnostics#9725

Open
AaronAjose wants to merge 2 commits intostarkware-libs:mainfrom
AaronAjose:fix/parser-consecutive-comparison-diagnostic-hints
Open

fix(parser): clarify consecutive comparison diagnostics#9725
AaronAjose wants to merge 2 commits intostarkware-libs:mainfrom
AaronAjose:fix/parser-consecutive-comparison-diagnostic-hints

Conversation

@AaronAjose
Copy link
Copy Markdown
Contributor

Clarify E1028 for consecutive comparison operators by adding conditionalguidance for chained comparisons and possible missing :: in generic paths, and update parser/semantic diagnostic test data with an additional 1 == 1 == 1 case.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

@orizi made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on AaronAjose).


crates/cairo-lang-parser/src/diagnostic.rs line 225 at r1 (raw file):

            ParserDiagnosticKind::ConsecutiveMathOperators { first_op, second_op } => {
                format!(
                    "Consecutive comparison operators are not allowed: {} followed by {}. If this \

since you are separating cases - the comment should probably be actually based on the operator.
otherwise just much more messy with no benefit.

@AaronAjose
Copy link
Copy Markdown
Contributor Author

@orizi Done, please re-check, thanks for your time.

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

@orizi reviewed all commit messages and made 2 comments.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on AaronAjose).


crates/cairo-lang-parser/src/diagnostic.rs line 235 at r2 (raw file):

                    )
                } else {
                    format!(

i


crates/cairo-lang-parser/src/diagnostic.rs line 239 at r2 (raw file):

                         `&&` (for example, `a < b && b < c`)."
                    )
                }

this does not make sense at the general case.
should be only on a a < b < c.

Code quote:

                } else {
                    format!(
                        "{message} If this was intended as a chained comparison, rewrite it with \
                         `&&` (for example, `a < b && b < c`)."
                    )
                }

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.

3 participants