Add lint preventing EIP references in backticks - Githubissues

ethereum / eipw

Mozilla Public License 2.0

27 stars 26 forks source link

Add lint preventing EIP references in backticks #90

Closed SamWilsn closed 1 month ago

SamWilsn commented 7 months ago

Many authors attempt to circumvent the EIP link requirements by putting EIP-XXXX in backticks (eg. `EIP-1234`.) This is bad form, and should be rejected.

VictoriaAde commented 5 months ago

@SamWilsn I'd like to work on this. Can I be assigned?

SamWilsn commented 5 months ago

Done! Be aware that we need to be careful to allow things like ERC20 or IERC7777 because they're interface names, but prevent EIP-7777 because that isn't valid code.

VictoriaAde commented 5 months ago

Noted!

Hari-Bombon commented 5 months ago

@SamWilsn I would love to work on this repo. Could you please assign me?

SamWilsn commented 5 months ago

@Hari-Bombon we should give the user above a chance to work on this, since she asked first. #11 or #5 might be good candidates to try instead?

VictoriaAde commented 5 months ago

Hi @SamWilsn, I have added the lint rule but my test keeps failing. What do you think I'm doing wrong?

SamWilsn commented 5 months ago

@VictoriaAde I'd need to see a bit more of your code (do you have a branch somewhere?) but if I had to guess, it looks like your src variable isn't a correctly formatted EIP document. It's missing the --- somewhere.

VictoriaAde commented 5 months ago

@SamWilsn I looked at what you said and now if I run cargo run or test, it will giving the below message. <SOURCE>... I can't seem to figure out what the issue is.

SamWilsn commented 5 months ago

Right, forgot about that. The project is divided into a few crates (aka packages):

eipw is at the root of the repository, and provides a command-line interface to the tool.
eipw-preamble is a library that provides functions for parsing the preamble of EIPs (the stuff at the beginning of proposals between --- lines.)
eipw-lint is where the majority of the interesting code lives, including the lints themselves.
eipw-lint-js is the glue that connects to JavaScript, and will likely be uninteresting for this issue.

To run the tests for lints, you'll need to change directory into eipw-lint, and run the tests there:

cd eipw-lint
cargo test

Alternatively, you can run all the tests from the root directory:

cargo test --all

Regarding the <SOURCES>... message, the tool is asking for the file you'd like to check. For example:

cargo run -- ~/EIPs/EIPS/eip-3074.md

VictoriaAde commented 5 months ago

Oh my, thank you so much. I will try these out.

VictoriaAde commented 4 months ago

@SamWilsn, I made a PR, please check it out