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

Permit impl item to end with semicolon #118

Merged
merged 3 commits into from
Dec 6, 2021

Conversation

dtolnay
Copy link
Contributor

@dtolnay dtolnay commented Aug 29, 2021

Writing signatures of a trait impl with a trailing semicolon is ubiquitous when discussing library APIs in Rust.

impl<'a, K: Debug   Ord, V: Debug> Error for OccupiedError<'a, K, V>;
                                                                    ^

Despite being not compiler-accepted syntax, the notation is easily understandable to human Rust programmers.

Some examples in the wild:

In fact the notation serves an important role in API design discussions: talking about impl Trait for SomeType; makes no claim about how the methods of Trait are implemented, just that Trait is implemented. How it's implemented is unspecified. This is in contrast to someone having written impl Trait for SomeType {} which is legal syntax for an impl that inherits all default method implementations from the trait definition, which is most often not what the participant intended to communicate.

// I propose adding this API to the standard library!

trait SomeTrait {
    fn method(&self) -> Option<&T> { /* default impl returns None */ }
}

// we'll provide the obvious SomeTrait impl for Thing
impl SomeTrait for std::thing::Thing;

// confusing: am I saying we will *not* provide a Thing-specific implementation of `method`?
impl SomeTrait for std::thing::Thing {}

Sadly, today the notation throws off tree-sitter. You can tell from GitHub's syntax highlighting in the links above. Notice how impl keywords after a semicolon aren't getting colored as keywords.



Running tree-sitter on the trait impl signatures added in this PR to corpus/declarations.txt confirms that tree-sitter doesn't really have that much idea what is going on. Before:

(source_file
  (ERROR
    (type_parameters
      (constrained_type_parameter
        (type_identifier)
        (trait_bounds (type_identifier) (type_identifier))))
    (identifier)
    (ERROR (generic_type (type_identifier) (type_arguments (type_identifier))))
    (abstract_type
      (ERROR
        (bracketed_type
          (ERROR (type_identifier))
          (bounded_type (type_identifier) (type_identifier)))
        (identifier))
      (generic_type (type_identifier) (type_arguments (type_identifier)))))
  (empty_statement))

This PR makes the body field of impl_item optional, and will parse impl signatures with a trailing semicolon as having no body.

(source_file
  (impl_item
    (type_parameters
      (constrained_type_parameter
        (type_identifier)
          (trait_bounds (type_identifier) (type_identifier))))
    (type_identifier)
    (generic_type (type_identifier) (type_arguments (type_identifier))))
  (impl_item
    (type_parameters
      (constrained_type_parameter
        (type_identifier)
        (trait_bounds (type_identifier) (type_identifier))))
    (type_identifier)
    (generic_type (type_identifier) (type_arguments (type_identifier)))))

@maxbrunsfeld maxbrunsfeld merged commit 0c250fc into tree-sitter:master Dec 6, 2021
@maxbrunsfeld
Copy link
Contributor

Sorry for the delay on this. Thanks for the great explanation.

@dtolnay dtolnay deleted the implsignature branch December 6, 2021 05:00
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.

2 participants