Skip to content

SONARPY-345 Parser should parse Python3 by default and fallback to Python2#252

Merged
andrea-guarino-sonarsource merged 2 commits intomasterfrom
SONARPY-345
Jul 19, 2019
Merged

SONARPY-345 Parser should parse Python3 by default and fallback to Python2#252
andrea-guarino-sonarsource merged 2 commits intomasterfrom
SONARPY-345

Conversation

@pynicolas
Copy link
Copy Markdown
Contributor

The other commit comes from #251

@pynicolas pynicolas force-pushed the SONARPY-345 branch 3 times, most recently from 25d55f7 to 0ba0588 Compare July 18, 2019 13:34
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 think there is a typo in the message of the second commit (SONARPY-245 instead of SONARPY-345). Btw, maybe we should create a separate ticket for the update of PrintStatementUsageCheck: what do you think?

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.

What do you think about adding DEBUG log message?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had to rebase against the master. Now, we need both parsers to compute metrics. I had to restructure the code.

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.

There are two code smells regarding errorElements and parseAs not being static

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

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.

Could you please add a comment explaining why we need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@pynicolas
Copy link
Copy Markdown
Contributor Author

I fixed the typo in the commit message.
I don't think we should create a separate ticket for PrintStatementUsageCheck:

  • there should be no change in behavior compared to what we had before
  • I had to update PrintStatementUsageCheck precisely because of the first commit: in fact, unit tests are red after the 1st commit because of a test file using a print statement.

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.

LGTM

@andrea-guarino-sonarsource andrea-guarino-sonarsource merged commit 8b372ca into master Jul 19, 2019
@andrea-guarino-sonarsource andrea-guarino-sonarsource deleted the SONARPY-345 branch July 19, 2019 13:30
hashicorp-vault-sonar-prod Bot pushed a commit that referenced this pull request May 8, 2025
…ClassType.resolveMember (#252)

GitOrigin-RevId: 1639651f5c931a8fb4ab21ab1432b15291cd534a
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