Skip to content

Integrate with Solium linter #17

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

Merged
merged 5 commits into from
Dec 25, 2017
Merged

Integrate with Solium linter #17

merged 5 commits into from
Dec 25, 2017

Conversation

LefterisJP
Copy link
Collaborator

@denisglotov this one is for you

This PR adds integration of Emacs solidity mode and flycheck with the solium linter.

At this point you now need to specify the path to the solc or the path to the solium binary via
solidity-solc-path or solidity-solium-path respectively. Reason is that in some configurations, solc may be added to the path after emacs is launched (e.g. systemd) so it's always good practise to have it explicitly specified.

Moreover since now we can have multiple flycheck checkers you need to specify the active flycheck checker. That is done via the solidity-flycheck-active-checker option which can either be "solc" or "solium".

Finally check the README for possible configuration options of solium and solc.

@LefterisJP LefterisJP merged commit a1bc9ca into master Dec 25, 2017
@LefterisJP LefterisJP deleted the integrate_solium branch December 25, 2017 18:51
@LefterisJP LefterisJP changed the title Intergrate with Solium linter Integrate with Solium linter Dec 25, 2017
@denisglotov
Copy link

@LefterisJP, I'd say that solidity-checker should be split to solidity-solc-checker and solidity-solium-checker, so that they could work together! Ask yourself: do you want to keep switching your checkers to see all errors that your edited code currently have.
That's why I stay with my solc-solium-wrapper.sh hack. But I began to see this message Solidity Mode Configuration error. Requested solc flycheck integration but can't find solc at: solc, do you know why?

@LefterisJP
Copy link
Collaborator Author

There are now 2 different checkers.

One is for solc and one is for solium. From what I saw solium is a superset of solidity, so having them both on would be a lot of noise so I am not letting it right now. If that's not the case I can change it.

The error you got is because you need to provide the full patch to solc now as per the readme.

@denisglotov
Copy link

@LefterisJP, I am not heavily experienced in solium, but here is what I see:
if I introduce syntax error (e.g. function -> xxxfunction), both emit errors. But if I introduce logic error (e.g. change the name of a variable at declaration point but not at usage points), solium is OK with that, only solc complains (Error: Undeclared identifier). So I think they are needed both.

@LefterisJP
Copy link
Collaborator Author

I see. In that case I am enabling chaining of the checkers with a new PR: #18

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.

2 participants