protofire / solhint

Solhint is an open-source project to provide a linting utility for Solidity code.
https://protofire.github.io/solhint/
MIT License
1.04k stars 160 forks source link

Becoming ESLint-like #5

Open sohkai opened 7 years ago

sohkai commented 7 years ago

Just to get the discussion rolling, I was wondering if it might make sense to conform to some of ESLint's CLI and options given how this project uses a lot of the great ideas that have come out of ESLint.

Some ideas (that may be better as separate issues if agreed upon):

A lot of these features are either present or coming via solium@1, but I don't see much of a point in having both when solhint seems like a cleaner implementation already using antlr4.

idrabenia commented 6 years ago

Hi @sohkai,

Thank you for this feedback!

From my opinion first three points is interesting. But business value of its is not so much - how it required implementation. I think maybe we schedule its implementation in future.

Point #4 and #6 is awesome tasks for next release!

Point #5 is interesting I think make sense to schedule it in future.

Last point from my opinion may increase complexity of config and soft. I am not sure that it make sense to decompose security to separate project when our linter is too small.

What do you think @sohkai? @fvictorio @mgarciap do you have any ideas?

fvictorio commented 6 years ago

Thanks a lot for the feedback, @sohkai. Some thoughts:

Change solhint.json to solhintrc.json Change init-config to --init

These two should be straightforward, right @idrabenia?

Add support for using 0, 1, and 2 as severity levels (or state that you can only use "off", "warn", "error")

This may be a little harder, and in my opinion using 0, 1, and 2 to mean "off", "warn", "error" is a bad practice, so I'd just add some clarification.

Add --quiet to only report errors

Do you mean to suppress the "extra" output of the program? Or to not report warnings?

Add support for custom rulesets, and inheriting from them Add support for custom plugins, and moving the security rules into a solhint-plugin-security-type package

I've still not dived into the code, so I don't know how much work this would take. But I agree that the "zero-config, plugins and bundles of plugins" seems to be the approach most tools are using (babel, eslint).

idrabenia commented 6 years ago

@fvictorio

"Change init-config to --init" From my opinion - It make sense to change init-config to new-config. Just --init I think is not have enough semantics to understand its' action.

"I've still not dived into the code, so I don't know how much work this would take. " It's not much work to make this changes. But from my understanding of software development - system must be as simple as possible. If we add additional complexity - we must clear understand - which benefits we will received from its.

I am not sure that it make sense to decompose security to separate plugin. I does not seeing any benefits for usability / maintainability / quality. But we will have development efforts and some additional actions will require for end-users to configure it.

I think it make sense just to provide possibilities to create plugins without decomposition of any rules to its.

- Ilya

sohkai commented 6 years ago

@idrabenia:

From my opinion - It make sense to change init-config to new-config. Just --init I think is not have enough semantics to understand its' action.

This isn't a big point, just a discrepancy I saw when comparing to ESLint. --init is generic, so it scales if any other configuration files (e.g. a .ignore) should be included in the future. But otherwise, I actually prefer init-config to new-config.

I am not sure that it make sense to decompose security to separate plugin. I does not seeing any benefits for usability / maintainability / quality. But we will have development efforts and some additional actions will require for end-users to configure it.

I think it make sense just to provide possibilities to create plugins without decomposition of any rules to its.

I like security as its own plugin because of separation of concerns: there are basic syntax and style linting rules, and then there are purpose-specific rules like those in security. Although I guess we could argue that security is a basic tenet of programming in solidity and so could be considered basic syntax ;). It's also just a good chance to dogfood the plugin infrastructure.

I haven't thought up of too many use cases for additional plugins, save some potential optimization checks (e.g. grouping unit8s), but I think it's an opportunity to let the community speak up. I'm sure ESLint weren't considering things like a11y and import when they built their plugin infrastructure, but then the community showed up to everyone's benefit.


@fvictorio:

Do you mean to suppress the "extra" output of the program? Or to not report warnings?

Just not report warnings. Useful sometimes, to not flood the output (e.g. when you have a lot of errors to fix).

in my opinion using 0, 1, and 2 to mean "off", "warn", "error" is a bad practice, so I'd just add some clarification.

Totally agree! Just a small note in the rules page should be fine.

mgarciap commented 6 years ago

Hi guys, what is this issue status? Shall it continue open?

fvictorio commented 6 years ago

None of the items listed was done, so I don't see why would we close it, except maybe to open a separate issue for each item.

GopherJ commented 2 years ago

I'm finding actually a perfect vim support for solhint and come across this. Feel it brings lots of value to be eslint-like so that solhint can benefit from eslint's ecosystem. Solhint can even become a plugin of eslint if possible.

I use coc-eslint (https://github.com/neoclide/coc-eslint) and did some search this morning to see if it's possible for coc-eslint to support solhint but seems not possible