Skip to content

SONARPY-407 Ensure nodes in the AST have the rightfirstToken and lastToken#317

Merged
benzonico merged 4 commits intomasterfrom
SONARPY-407
Sep 12, 2019
Merged

SONARPY-407 Ensure nodes in the AST have the rightfirstToken and lastToken#317
benzonico merged 4 commits intomasterfrom
SONARPY-407

Conversation

@andrea-guarino-sonarsource
Copy link
Copy Markdown
Contributor

No description provided.

@andrea-guarino-sonarsource andrea-guarino-sonarsource changed the title Sonarpy 407 SONARPY-407 Ensure nodes in the AST have the rightfirstToken and lastToken Sep 11, 2019
Copy link
Copy Markdown
Contributor

@benzonico benzonico left a comment

Choose a reason for hiding this comment

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

I believe you can also add a test about not having a first token null in one of the test of the base tree visitor. That would not be exhaustive but would still add a constraint to the tree.

* null if class is defined without args : class Foo:
* empty list if class defined with empty parentheses : class Foo():
* @return
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe we lack quite some documentation around tree interfaces, what about updating it explaining in which cases this part of the tree is null rather than simply dropping it ?

Copy link
Copy Markdown
Contributor

@benzonico benzonico left a comment

Choose a reason for hiding this comment

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

LGTM!

@benzonico benzonico merged commit 487b66a into master Sep 12, 2019
@benzonico benzonico deleted the SONARPY-407 branch September 12, 2019 06:23
hashicorp-vault-sonar-prod Bot pushed a commit that referenced this pull request Jun 6, 2025
GitOrigin-RevId: 6d1bb481a199fde32cfd7c564fa1ca284dce751c
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