Skip to content

Add linter rule for spell address comparison #7

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

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

KirillDogadin-std
Copy link
Contributor

Closes # .

Checklist:

  • issue number linked above after pound (#)
    • replace "Closes " with "Contributes to" or other if this PR does not close the issue
  • issue checkboxes are all addressed
  • manually checked my feature / not applicable
  • wrote tests / not applicable
  • attached screenshots / not applicable

@KirillDogadin-std
Copy link
Contributor Author

KirillDogadin-std commented Jul 24, 2023

Current state summary:

  • the rule seems to work when ran locally:
    • tsc && ./bin/lint.js <PATH_PREFIX_DEPENDS_ON_USER>/spells-mainnet/src/DssSpell.sol
    • the spells-mainnet branch is origin/2023-06-28
  • idea of the tool:
    • detect the description variable that is mandatory for the spell
    • get the line with the comment above this variable and expect the hash command to be provided there
    • extract the raw gh url from the hash command and download the file to the tempdir of the OS under the predefined / static name.
    • parse the source code to get the AST and find the nodes with addresses: get the list of the addressees.
    • parse the exec document with the regexp to get the addresses' list.
    • convert the lists to sets of lowercase strings - compare the sets
  • did not address test / integration at all.
  • outstanding points to address:
    • general code polishing and refactoring
    • adding tests
      • not only writing the test is required but also emulating/mocking the presence of the exec doc. Or storing the sample exec doc as test data.
    • fixing ci (if needed)
    • reconsidering the way the exec doc file is stored / handled.
    • looking for a better way to extract the addresses from the exec doc.

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.

1 participant