Catch unhandled panics in cadence lint and add proper account access checking #2227
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, when the Cadence semantic checker encountered certain code patterns (like optional chaining on reference types), it would panic instead of returning an error, causing the entire lint command to crash. This fix: 1. Adds panic recovery to gracefully handle internal Cadence checker errors 2. Implements proper MemberAccountAccessHandler that checks if contracts are deployed to the same account across networks (similar to PR #531 in cadence-tools for the language server) 3. Uses AccessCheckModeNotSpecifiedUnrestricted to be permissive during linting Now `flow cadence lint` always completes successfully and correctly validates access(account) usage based on deployment configuration.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
jribbink
added
Bugfix
Improvement
jribbink
changed the title
jribbink
requested review from
Kay-Zee,
bjartek,
chasefleming,
janezpodhostnik,
mfbz and
nvdtf
as code owners
chasefleming
approved these changes
Dec 19, 2025
There was a problem hiding this comment.
Pull request overview
This PR improves the robustness of the flow cadence lint command by adding panic recovery and implementing proper account access checking for Cadence contracts.
Key changes:
- Adds panic recovery in
lintFileto gracefully handle internal Cadence checker errors instead of crashing - Implements
MemberAccountAccessHandlerto validateaccess(account)usage based on deployment configuration across networks - Changes
AccessCheckModefromStricttoNotSpecifiedUnrestrictedto work with the new account access handler
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/cadence/linter.go | Adds panic recovery, implements account access checking logic with helper functions, and configures the semantic checker with the new handler |
| internal/cadence/lint.go | Converts internal errors from panic recovery into diagnostics to allow continued processing of remaining files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2228
Description
Previously, when the Cadence semantic checker encountered certain code patterns (like optional chaining on reference types), it would panic instead of returning an error, causing the entire lint command to crash.
This fix:
Now
flow cadence lintalways completes successfully and correctly validates access(account) usage based on deployment configuration.For contributor use:
masterbranchFiles changedin the Github PR explorer