juanfranblanco / vscode-solidity

Visual Studio Code language support extension for Solidity smart contracts in Ethereum https://marketplace.visualstudio.com/items?itemName=JuanBlanco.solidity
MIT License
892 stars 190 forks source link

Syntax error when emitting events in Solidity 0.4.21 #57

Closed ghost closed 6 years ago

ghost commented 6 years ago

This issue is a duplication of this question on Ethereum StackExchange:

I'm having a syntax error in VS Code in the parts where I emit events using new notation for Solidity v0.4.21.

Remix doesn’t raise any errors, though.

My User Settings in VS Code contain "solidity.compileUsingRemoteVersion": "latest", and compiler doesn't raise error on emit itself, rather gives this message:

Syntax error: Expected "!=", "%", "%=", "&", "&&", "&=", "*", "*=", "+", "++", "+=", ",", "-", "--", "-=", "/", "/*", "/=", ";", "<", "<<", "<<=", "<=", "=", "==", ">", ">=", ">>", ">>=", ">>>", "?", "^", "^=", "|", "|=", "||", comment, end of line, or whitespace but "(" found.

Here's how it looks like (2nd emit event is ok for some reason):

screen shot 2018-03-13 at 22 36 25

The problem is in syntax only, compilation completes successfully.

naddison36 commented 6 years ago

emit just needs to be added as a keyword in the following line https://github.com/juanfranblanco/vscode-solidity/blob/master/syntaxes/solidity.json#L76 @rrickdar do you want to have a go at doing a Pull Request for this change?

ghost commented 6 years ago

I see now that @lew made a Pull Request just two days ago: https://github.com/juanfranblanco/vscode-solidity/pull/56/

ImmuneGit commented 6 years ago

it doesn't resolves a problem for me... just change a color of emit from white to purple and I got: Syntax error: Expected "!=", "%", "%=", "&", "&&", "&=", "", "=", "+", "++", "+=", ",", "-", "--", "-=", "/", "/*", "/=", ";", "<", "<<", "<<=", "<=", "=", "==", ">", ">=", ">>", ">>=", ">>>", "?", "^", "^=", "|", "|=", "||", comment, end of line, or whitespace but "(" found.

lew commented 6 years ago

https://github.com/duaraghav8/solparse/issues/29

I think this was the problem

ghost commented 6 years ago

@ImmuneGit , updating npm packages solium to v1.5.5. and solparse to v2.2.4 solves the issue

ghost commented 6 years ago

As mentioned by @duaraghav8 in #https://github.com/duaraghav8/Solium/issues/177, the fix will be added in the new version of Solium, i.e the Solparse dependency where support for emit was recently added in v2.2.4 will be updated.

duaraghav8 commented 6 years ago

Hey guys, @rrickdar is correct, the latest solparse release 2.2.4 contains support for emit. But the latest Solium release 1.1.5 contains solparse 2.2.3. I have limited knowledge of VSCode extensions. If there is a trivial way for you to update just solparse for vscode-solidity, it will solve your problem instantly.

But if that's not the case, kindly wait for 3-4 days. I'm working on a few issues in solium and will be making a new release with the updated parser.

ghost commented 6 years ago

Updating solparse to 2.2.4 alone didn't help, rather update combination of solium 1.1.5 plus solparse 2.2.4 with a bit of manual modification of package-lock.json worked perfectly. Thanks everyone in this thread!

Afterword: of course, step 1 before any updates is to edit line 76 in syntaxes/solidity.json as mentioned by @naddison36 and @lew

duaraghav8 commented 6 years ago

^Thanks, I think you'd have to do that if the extension by default uses solium v0.

juanfranblanco commented 6 years ago

@rrickdar @duaraghav8 I have updated the extension to both the latest and works like a charm thanks and merged that pull request too.

juanfranblanco commented 6 years ago

@duaraghav8 BTW many thanks for Solium :)

duaraghav8 commented 6 years ago

Great @juanfranblanco! Thanks for your work on this & Nethereum :)

juanfranblanco commented 6 years ago

@duaraghav8 I think I spoken too soon ?

If I put emit I get an error. image

But if don't use emit I don't get an error I get a warning, which is the correct behaviour

image

It was not reported yesterday as the message format has changed (other issue)

duaraghav8 commented 6 years ago

@juanfranblanco actually this isn't unexpected. Vscode Solidity uses the latest version of Solparse (2.2.4) which has support for emit.

But this error came from Solium. The latest version of it (1.1.5) contains solparse 2.2.3 as an internal dependency, which does not have support for emit. So when Solium tries to lint your solidity code, it doesn't recognise emit because it uses its internal solparse which doesn't support emit.

This isn't your fault. I'll be releasing the next version of solium in a day or 2 which will contain latest solparse as internal dep. So all you need to do is update vscode-solidity's solium version to 1.1.6 when I ping you (on this thread or gitter channel?).

juanfranblanco commented 6 years ago

@duaraghav8 Excellent thanks, I am getting notifications too now. :)

juanfranblanco commented 6 years ago

Note: Notifications from Github. I had problems.

@duaraghav8 I have added autofix to the extension for the current document using Solium. I had a look at the indentation rules to implement fixing based on the "Quotes" one, but true, no idea where to start.

duaraghav8 commented 6 years ago

ah! The indentation rule is the longest (and most spaghetti!) code I've written in Solium. I wouldn't be surprised if somebody got lost trying to read it.

I'd recommend you don't implement indent's fix right now. Its on priority since manually fixing indents is obviously very painful. I plan to get 2-3 devs on board now that we have a little help from ethereum grants who'll refactor and implement all the missing fixes.

Nevertheless, if you'd still like to give it a shot, this section will help you understand how a rule is implemented in solium and contains info about implementing fix. From there, I'll help clarify all your doubts.

The list of rules currently providing autofix can be found here

juanfranblanco commented 6 years ago

@duaraghav8 Thanks, if you are getting developers, they will do a much better job than me. The implementation (whilst rudimentary) is already there to trigger it, I won't just mention it yet.

juanfranblanco commented 6 years ago

Referencing https://github.com/juanfranblanco/vscode-solidity/issues/18

duaraghav8 commented 6 years ago

MAN this feature is in demand!

DanielRX commented 6 years ago

@duaraghav8 Mainly because right now linting for 0.4.21 fails for me, so I can't actually use emit and have to ignore the warning that I need to add it, otherwise the linter just stops on the first use of emit

juanfranblanco commented 6 years ago

@DanielRX he was referring to the formatting, he mentioned he will be releasing the fix related to the emit soon.

And yes, the workaround is to ignore the warning for the time being. Good point you mention it.

DanielRX commented 6 years ago

@juanfranblanco Apologies, is there anyway I can edit the deps myself so that I can have solium run with the latest solparse just as a bootstrap so I can avoid forgetting to readd emits?

juanfranblanco commented 6 years ago

well another option is to modify the package file of solium.

" "dependencies": { "ajv": "^5.2.2", "chokidar": "^1.6.0", "colors": "^1.1.2", "commander": "^2.9.0", "js-string-escape": "^1.0.1", "lodash": "^4.14.2", "sol-digger": "0.0.2", "sol-explore": "1.6.1", "solium-plugin-security": "0.1.1", "solparse": "2.2.4", "text-table": "^0.2.0" },"

But I cannot release this as the packaging resets all the package references. (Ill try again)

juanfranblanco commented 6 years ago

Ok I have actually released a new version with the dependency hardcoded, test it and see how it goes @DanielRX

DanielRX commented 6 years ago

@juanfranblanco I think it's working, now I just need to work out a nice way to edit my line endings when I swap windows <-> unix until that fix is out aha

duaraghav8 commented 6 years ago

@juanfranblanco Solium 1.1.6 is now out and supports emit. I haven't included support for automatic indentation formatting yet, turns out to be way more complex than I thought! Let me know if you face any issues

juanfranblanco commented 6 years ago

@duaraghav8 Many thanks, I will update it today and let you know. And yes indentation formatting seems rather complex.

elie222 commented 6 years ago

So I don't have problems with emit anymore, but I do have an issue when I use msg.sender on a line that has emit in it:

ParserError: Expected token Semicolon got 'LParen'
    emit Deposit(msg.sender, _amount, shares, totalShares);
juanfranblanco commented 6 years ago

@duaraghav8 FYI ^^

juanfranblanco commented 6 years ago

@elie222 I don't see any issues here: image

duaraghav8 commented 6 years ago

@juanfranblanco yep I saw it, tested on my local, couldn't reproduce the issue (solium 1.1.6). @elie222 would you mind producing your whole code here?

elie222 commented 6 years ago

A bunch changed in my settings and the issue has gone away since then. I think there may be some issues for users when using both this package and the extended version of the package together. I had some weird issues around linting and emit going on and had both packages installed.

juanfranblanco commented 6 years ago

yes having the extended version has been validated to be a problem.