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

Becoming ESLint-like #5

Open
2 of 7 tasks
sohkai opened this issue Oct 26, 2017 · 7 comments
Open
2 of 7 tasks

Becoming ESLint-like #5

sohkai opened this issue Oct 26, 2017 · 7 comments
Labels
help wanted Team would love to have contributors working on these

Comments

@sohkai
Copy link
Contributor

sohkai commented Oct 26, 2017

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):

  • Change solhint.json to solhintrc.json (we could use cosmiconfig for this)
  • Change init-config to --init
  • Add support for using 0, 1, and 2 as severity levels (or state that you can only use "off", "warn", "error") (see also Disable rules using "off" instead of "false" #43)
  • Add --quiet to only report errors
  • Add support for a .ignores-type file
  • 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

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
Copy link
Contributor

idrabenia commented Dec 2, 2017

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
Copy link
Contributor

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
Copy link
Contributor

idrabenia commented Dec 5, 2017

@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
Copy link
Contributor Author

sohkai commented Dec 17, 2017

@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 mgarciap added the help wanted Team would love to have contributors working on these label Apr 25, 2018
@mgarciap
Copy link
Contributor

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

@fvictorio
Copy link
Contributor

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
Copy link

GopherJ commented Jul 16, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Team would love to have contributors working on these
Projects
None yet
Development

No branches or pull requests

5 participants