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

Grammar doesn't understand function's array return value being accessed in the same instruction #749

Closed
yadimon opened this issue Aug 20, 2015 · 11 comments
Assignees
Labels
antlr Issue is easier to resolve with knowledge of Antlr4 bug Identifies work items for known bugs critical Marks a bug as a must-fix, showstopper issue
Milestone

Comments

@yadimon
Copy link
Contributor

yadimon commented Aug 20, 2015

Hi, I tried some RD function over some projects,
the parser shows error, but for different lines, somewhere near the code examples.
The following code examples generates errors for Parser:

somevar = Split(anyvar, "anystring")(0) (the last part is the problem)

and

Dir$

@yadimon yadimon changed the title Parser error on Split(x, "x")(0) and Dir$ Parser error on Split(variable, "text")(0) and Dir$ Aug 20, 2015
@rubberduck203
Copy link
Member

Any chance you have the error message from the parser handy?

@rubberduck203 rubberduck203 added bug Identifies work items for known bugs parse-tree-processing labels Aug 20, 2015
@retailcoder
Copy link
Member

The grammar doesn't understand that a function can return an array that's accessed immediately; the grammar rule allows for chained member calls, but the array or procedure call rule is currently standing on its own, which means...

Repro:

Sub Foo()
    bar = Split("abc,def", ",")(0)
End Sub

Work-around:

Sub Foo()
    temp = Split("abc,def", ",")
    bar = temp(0)
End Sub

By splitting the call on two instructions, the grammar rules are satisfied and the parser understands what's going on.

Not ideal, but note taken - we'll see how the grammar can be tweaked to understand that an array call can be made there.

@retailcoder retailcoder added antlr Issue is easier to resolve with knowledge of Antlr4 and removed parse-tree-processing labels Aug 20, 2015
@retailcoder retailcoder changed the title Parser error on Split(variable, "text")(0) and Dir$ Grammar doesn't understand function's array return value being accessed in the same instruction Aug 20, 2015
@retailcoder retailcoder self-assigned this Aug 20, 2015
@yadimon
Copy link
Contributor Author

yadimon commented Aug 20, 2015

and what about Dir$ error?

Sub Foo()
    Dir$ "/"
End Sub

@retailcoder
Copy link
Member

@yadimon Interesting. The same code using Dir instead of Dir$ doesn't blow up the parser.

My guess is that the $ trips the parser because in this context Dir is interpreted as a procedure call, and it seems only a function call is legal with a type hint - this code doesn't blow up the parser:

Sub Foo()
    bar = Dir$("/")
End Sub

We might need to special-case Dir$.

@manfredk
Copy link

Just to stress the importance of this issue:

I tried Rubberduck with a medium complex project where I'm using this construct (function's array return value immediately indexed) quite frequently. Unfortunately the parser breaks, showing only the first parse error and parsing would never come to an end (showing VBProject parsing ... indefinitely, refresh not working). No time to change dozens of lines just to get code explorer working.

But Rubberduck seems very promising and a quantum leap in vba coding assistance. I'll definitely take a closer look and dive into debugging RD itself, meanwhile KEEP GOING!

Cheers

@retailcoder retailcoder added the critical Marks a bug as a must-fix, showstopper issue label Sep 13, 2015
@retailcoder
Copy link
Member

@manfredk thanks! I see you forked the project - feel free to contact me anytime if you have questions... I haven't really documented how to compile the ANTLR grammar and re-generate the parser and other needed base classes... I need to do that.

I'm currently working on reconnecting the pieces (see #759), and that a lot of the issues linked there are "up-for-grabs", as a result of #606. In the process, we're swapping some WinForms UI for some WPF/XAML, toward a MVVM approach. This change is far from being just cosmetic, and significantly simplifies and organizes the code base... on top of being more elegant in both Visual Studio and the VBE.

I was planning on implementing the grammar fixes after I get the code inspections and regex search/replace up, so a number of weeks still; I'll gladly accept a PR that fixes grammar issues before I get to them! 😃

@manfredk
Copy link

Hi Mathieu,

I did a project with VSTO in 2013 (Excel only), but Rubberduck is far more
complex. I'm still wrapping my head around some concepts which are at the
core of rubberduck (Parsing with Antlr), so probably no PR from my side
concerning this issue. Currently I'm busy with a project, but I hope to
dive deeper into Rubberduck in 2 months. Would you need someone for
translating the german locale ? ... I'd be ready to do that in November.

Again, great work !!. I'm curious about v2.

Cheers, Manfred

On Sun, Sep 13, 2015 at 5:40 AM, Mathieu Guindon notifications@github.com
wrote:

@manfredk https://github.com/manfredk thanks! I see you forked the
project - feel free to contact me anytime if you have questions... I
haven't really documented how to compile the ANTLR grammar and re-generate
the parser and other needed base classes... I need to do that.

I'm currently working on reconnecting the pieces (see #759
#759), and that a
lot of the issues linked there are "up-for-grabs", as a result of #606
#606. In the
process, we're swapping some WinForms UI for some WPF/XAML, toward a MVVM
approach. This change is far from being just cosmetic, and
significantly simplifies and organizes the code base... on top of being
more elegant in both Visual Studio and the VBE.

I was planning on implementing the grammar fixes after I get the code
inspections and regex search/replace up, so a number of weeks still; I'll
gladly accept a PR that fixes grammar issues before I get to them! [image:
😃]


Reply to this email directly or view it on GitHub
#749 (comment)
.


Manfred Kipfelsberger, Schwabacher Str. 129, 90763 Fürth
web: http://www.cen-seo.de, email: mk@cen-seo.de
Tel. 0151-270 20 866, skype: manfred.kipfelsberger


@Vogel612
Copy link
Member

Hi @manfredk,

this is the current translator for German locale speaking 😄
Amendmends and improvement ideas are very welcome.

You can find the current Translation under Retailcoder.VBE/UI/RubberduckUI.de.resx

If you need something to make this nasty XML file look better, you may want to check
https://github.com/Vogel612/TranslationHelper or https://github.com/Hosch250/ResxEditor

Greets, Vogel612

@rubberduck203
Copy link
Member

Hey there @manfredk. I can completely understand having other things to do, but don't let ANTLR scare you away. You couldn't fix this issue without learning a bit about it, but there are lots of other areas that could use some TLC. All PRs are considered, we're always happy for any help we get. We hope that you'll come back around down the road and contribute.

Cheers

@manfredk
Copy link

@Vogel612 Hi Clemens, good to know, I'll have a look.

@retailcoder
Copy link
Member

Looks like this issue is now fixed on my fork and will be closed when I merge my changes in 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
antlr Issue is easier to resolve with knowledge of Antlr4 bug Identifies work items for known bugs critical Marks a bug as a must-fix, showstopper issue
Projects
None yet
Development

No branches or pull requests

5 participants