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

Syntax error when emitting events in Solidity 0.4.21 #57

Closed
ghost opened this issue Mar 14, 2018 · 34 comments
Closed

Syntax error when emitting events in Solidity 0.4.21 #57

ghost opened this issue Mar 14, 2018 · 34 comments

Comments

@ghost
Copy link

ghost commented Mar 14, 2018

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

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

ghost commented Mar 14, 2018

I see now that @lew made a Pull Request just two days ago:
#56

@ImmuneGit
Copy link

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

lew commented Mar 16, 2018

duaraghav8/solparse#29

I think this was the problem

@ghost
Copy link
Author

ghost commented Mar 16, 2018

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

@ghost
Copy link
Author

ghost commented Mar 16, 2018

As mentioned by @duaraghav8 in #duaraghav8/Ethlint#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
Copy link

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

ghost commented Mar 16, 2018

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

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

@juanfranblanco
Copy link
Owner

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

@juanfranblanco
Copy link
Owner

@duaraghav8 BTW many thanks for Solium :)

@duaraghav8
Copy link

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

@ghost ghost closed this as completed Mar 17, 2018
@juanfranblanco
Copy link
Owner

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

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

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

@juanfranblanco
Copy link
Owner

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

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

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

Referencing #18

@duaraghav8
Copy link

MAN this feature is in demand!

@DanielRX
Copy link

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

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

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

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

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

@DanielRX
Copy link

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

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

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

@elie222
Copy link

elie222 commented Apr 4, 2018

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

@duaraghav8 FYI ^^

@juanfranblanco
Copy link
Owner

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

@duaraghav8
Copy link

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

elie222 commented Apr 10, 2018

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

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

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

No branches or pull requests

7 participants