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

[lint] Add Cadence 1.0 analyzer with basic linting rules #285

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

jribbink
Copy link
Contributor

@jribbink jribbink commented Feb 7, 2024

Closes #287

Description

This PR adds a cadence 1.0 analyzer with initial linting rules which identify some instances of pre-Cadence 1.0 code. These diagnostics will display more verbose errors than syntax errors reported by the checker or lexical errors reported by the parser, potentially provide automatic fixes, as well as direct users to relevant documentation regarding how to upgrade their code.

This PR depends on the 2 following PRs:

and will not be valid until these PRs are merged & this PR is updated to the latest version of Cadence


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice!

lint/analyzers_test.go Outdated Show resolved Hide resolved
lint/cadence_v1_analyzer.go Show resolved Hide resolved
lint/test.cdc Outdated Show resolved Hide resolved
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple remaining questions re: the parser/checker error handlers

lint/analyzers_test.go Outdated Show resolved Hide resolved
lint/analyzers_test.go Outdated Show resolved Hide resolved
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

@SupunS could you please also have a look?

Copy link
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

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

Nice!

lint/cadence_v1_analyzer.go Outdated Show resolved Hide resolved
lint/cadence_v1_analyzer.go Show resolved Hide resolved
lint/cadence_v1_analyzer.go Outdated Show resolved Hide resolved
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice work!

@turbolent turbolent merged commit dff8cfa into master Feb 16, 2024
6 checks passed
@turbolent turbolent deleted the jribbink/c1-warnings branch February 16, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[lint] Add Cadence 1.0 Analyzer
3 participants