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

fix: improve solhint custom rules and remove TODOs #5497

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

crStiv
Copy link

@crStiv crStiv commented Feb 11, 2025

This commit improves the solhint custom rules by:

  1. Expanding visibility check for constant variables to include internal constants
  2. Re-enabling and fixing the no-external-virtual rule with:
    • Skip checks for constructors and interfaces
    • Improved error message with actionable suggestion
    • Fixed comparison operator

Both TODO comments have been addressed while maintaining the codebase's
coding standards and improving the overall code quality checks.

Copy link

changeset-bot bot commented Feb 11, 2025

⚠️ No Changeset found

Latest commit: 4f95017

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Amxx Amxx self-requested a review February 11, 2025 19:59
@Amxx Amxx added the CI label Feb 11, 2025

FunctionDefinition(node) {
// Skip constructor, function without a name (receive/fallback) and interface definitions
if (node.isConstructor || node.name == null || node.parent.kind === 'interface') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

NOTE: prettier rules say that receive and fallback function MUST be external. This creates a conflicts if they are virtual. This leaves us tree options:

  • remove virtual from receive/fallback: bit no for me!
  • change the prettier config
  • (what I did here) skip the rule for receive and fallback functions.

@Amxx Amxx modified the milestones: 5.3, 5.4 Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants