Code completion ignores local variables in mapping operation

[eclipse-qvt-oml-bot]

| --- | --- | | Bugzilla Link | 296630 | | Status | RESOLVED FIXED | | Importance | P3 normal | | Reported | Dec 01, 2009 15:51 EDT | | Modified | Mar 19, 2013 05:32 EDT | | Version | 2.0 | | Blocks | 398009 | | Reporter | Radomil Dvorak |

Description

The issue may be reproduced by the code snippet below.\ Interestingly, if this operation is defined as helper,\ all works fine.

mapping FOOContainer::xxxxx() : FOOContainer {\ var s:String;\ s. // no proposals\ }

[eclipse-qvt-oml-bot]

By Christopher Gerking on Sep 06, 2012 17:54

Created attachment 220812 (attachment deleted)\ Patch enabling local variable proposals inside mappings

The bug is in ScopedVariablesExtractor.extractVariables. The '{' token at the beginning of a body is required to trigger the analysis for scoped variables, but this token was skipped for mappings up to now.

[eclipse-qvt-oml-bot]

By Christopher Gerking on Oct 17, 2012 06:21

Created attachment 222457 Patch enabling local variable proposals inside mappings (updated copyright and contributors)

:notepad_spiral: ScopedVariablesExtractor.patch

[eclipse-qvt-oml-bot]

By Adolfo Sanchez-Barbudo Herrera on Jan 24, 2013 08:45

+1.

Trivial comment (no action needed):

• Copyright header is not the proper one.

Cheers,\ Adolfo.

[eclipse-qvt-oml-bot]

By Adolfo Sanchez-Barbudo Herrera on Mar 11, 2013 06:52

Patch with Copyright fix pushed to master for Kepler M6.

N.B: Since we don't have any M6 or 3.3 M6 flag, I've removed the target milestone. Any guidance about how to modify that ?

[eclipse-qvt-oml-bot]

By Adolfo Sanchez-Barbudo Herrera on Mar 11, 2013 10:03

Reopening...

QVTo UI test cases launching is producing some failures which need to be sorted out.

[eclipse-qvt-oml-bot]

By Adolfo Sanchez-Barbudo Herrera on Mar 11, 2013 11:22

Christopher,

The solution is not definitely the good one....

Indeed, the following example now works with the patch and it didn't previously do:

mapping FOOContainer::xxxxx() : FOOContainer {\ var s : String;\ s.\ }

However, the following example doesn't work now and it previously did:

mapping FOOContainer::xxxxx() : FOOContainer {

init { var s : String; }\
\
population { s. }\

}

The test cases detect other failures. So this patch, while it cures one case, it breaks others. So the commit will have to be reverted.

After debugging it looks like that in the original case, "s" can't be resolved of a variable hence it's evaluated as "invalid", hence, no String operations appear after all.... On the other hand, the following case currently works:

mapping FOOContainer::xxxxx() : FOOContainer {\ population {\ var s : String;\ s.\ }\ }

Which is semantically the same as the original example (the expressions written in the original code, belong to the population section of the mapping) .

I don't know how this CodeCompletion, "LightWeight" parser and such work at all and I don't want to spend more time in debugging it to understand it since this issue should be automatically dealt by Xtext editors in the future.

If you provide a better patch, with a test case and no failures of the QVTo Ui tests.... We could reconsider fixing this issue.

Cheers,\ Adolfo.

[eclipse-qvt-oml-bot]

By Adolfo Sanchez-Barbudo Herrera on Mar 11, 2013 11:50

Comment on attachment 222457 Patch enabling local variable proposals inside mappings (updated copyright and contributors)

Removing ip

:notepad_spiral: ScopedVariablesExtractor.patch

[eclipse-qvt-oml-bot]

By Ed Willink on Mar 12, 2013 02:44

(In reply to comment #6)

I don't know how this CodeCompletion, "LightWeight" parser and such work at all and I don't want to spend more time in debugging it to understand it since this issue should be automatically dealt by Xtext editors in the future.

Indeed, with Xtext you start with something grammar+metamodel driven and optionally tune specific responses.

The current code has nearly 20% of its testing specifically targetted at the heuristic heroics. Quite what is a good proposal is sometimes a bit subjective, so the test failures may not necessarily be bad.

-1 for now, but if Christopher wants to analyze and justify the failures we could choose to accept the change after all.

[eclipse-qvt-oml-bot]

By Adolfo Sanchez-Barbudo Herrera on Mar 12, 2013 06:46

(In reply to comment #8)\

-1 for now, but if Christopher wants to analyze and justify the failures we could choose to accept the change after all.

The failures can't be justified. Another trivial to check regression:

mapping FOOContainer::xxxxx() : FOOContainer {\ init { var s : String; }\ population { }\ }

With the patch the s var is not proposed in the popuation section. Without the patch the var s is proposed.

Cheers,\ Adolfo.

[eclipse-qvt-oml-bot]

By Sergey Boyko on Mar 15, 2013 06:11

Alternative fix that correctly deals with different mapping's composition.

[eclipse-qvt-oml-bot]

By Adolfo Sanchez-Barbudo Herrera on Mar 15, 2013 07:21

Great.

The tests I had in my second instance work now. To me the fix is good.

Cheers,\ Adolfo.

[eclipse-qvt-oml-bot]

By Sergey Boyko on Mar 16, 2013 05:41

Pushed to master for Kepler M6.

Commit ID: f946c2b320fea41667dbb11db75b4e646b7587d3

[eclipse-qvt-oml-bot]

By Philipp W. Kutter on Mar 19, 2013 01:45

What is missing to put this to verified?

[eclipse-qvt-oml-bot]

By Ed Willink on Mar 19, 2013 05:32

See my response on https://bugs.eclipse.org/bugs/show_bug.cgi?id=309762#c20.

Please do not waste our time by spamming Bugzillas. We have a developers mailing list (https://dev.eclipse.org/mailman/listinfo/qvto-dev) that is much more suitable for philosophiocal and project discussions.