Skip to content

Consider : to terminate parenthesized with items#11775

Merged
dhruvmanila merged 1 commit intomainfrom
dhruv/with-items
Jun 6, 2024
Merged

Consider : to terminate parenthesized with items#11775
dhruvmanila merged 1 commit intomainfrom
dhruv/with-items

Conversation

@dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jun 6, 2024

Summary

This PR is a follow-up to this discussion (#11770 (comment)) which adds the : token in the terminator set for parenthesized with items.

The main motivation is to avoid parsing too much in speculative mode. This is evident with the following before and after parsed with items list for the following code:

with (item1, item2:
    foo
Before (3 items) After (2 items)
parsed_with_items: [
    ParsedWithItem {
        item: WithItem {
            range: 6..11,
            context_expr: Name(
                ExprName {
                    range: 6..11,
                    id: "item1",
                    ctx: Load,
                },
            ),
            optional_vars: None,
        },
        is_parenthesized: false,
    },
    ParsedWithItem {
        item: WithItem {
            range: 13..18,
            context_expr: Name(
                ExprName {
                    range: 13..18,
                    id: "item2",
                    ctx: Load,
                },
            ),
            optional_vars: None,
        },
        is_parenthesized: false,
    },
    ParsedWithItem {
        item: WithItem {
            range: 24..27,
            context_expr: Name(
                ExprName {
                    range: 24..27,
                    id: "foo",
                    ctx: Load,
                },
            ),
            optional_vars: None,
        },
        is_parenthesized: false,
    },
]
parsed_with_items: [
    ParsedWithItem {
        item: WithItem {
            range: 6..11,
            context_expr: Name(
                ExprName {
                    range: 6..11,
                    id: "item1",
                    ctx: Load,
                },
            ),
            optional_vars: None,
        },
        is_parenthesized: false,
    },
    ParsedWithItem {
        item: WithItem {
            range: 13..18,
            context_expr: Name(
                ExprName {
                    range: 13..18,
                    id: "item2",
                    ctx: Load,
                },
            ),
            optional_vars: None,
        },
        is_parenthesized: false,
    },
]

Test Plan

cargo insta test

@dhruvmanila dhruvmanila added internal An internal refactor or improvement parser Related to the parser labels Jun 6, 2024
@dhruvmanila dhruvmanila requested a review from MichaReiser as a code owner June 6, 2024 11:44
@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@dhruvmanila dhruvmanila merged commit 1b7d08c into main Jun 6, 2024
@dhruvmanila dhruvmanila deleted the dhruv/with-items branch June 6, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal An internal refactor or improvement parser Related to the parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants