Review Request 118575: QML components can only see the declarations of their base classes and in the top-level context, not their parents

[Denis Steckelmacher]

> On June 5, 2014, 9:36 p.m., Sven Brauch wrote:
> > One important thing to check is to let it parse something and then restart KDevelop a few times and watch out for crashes or weird behaviour. You won't notice itemrepository-related bugs so easily in a test or a single run. If that works, I'd say what you have looks ok ;)
> > 
> > I'm not fully convinced whether overriding the behaviour of the well-known functions is the nicest way to do this. Couldn't you provide a new function, say, visibleDeclarations(...) and call that instead? Or are there other disadvantages to doing that?

In order not to duplicate most of DUContext, a visibleDeclarations() method would have to call findDeclarations or allDeclarations. The problem is that it is the behavior of these two methods that must be changed, so simply wrapping them around would not work. The fact that they are virtual explicitly says "change behavior here", and by doing this, I'm sure that the correct behavior will always be used, even if KDevPlatform itself looks for declarations (thus not by using visibleDeclarations()).


- Denis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118575/#review59362
-----------------------------------------------------------


On June 5, 2014, 9:15 p.m., Denis Steckelmacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118575/
> -----------------------------------------------------------
> 
> (Updated June 5, 2014, 9:15 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-qmljs
> 
> 
> Description
> -------
> 
> QML has very strange visibility rules. Here is an example:
> 
> import QtQuick.Controls 1.1
> 
> Label {
>   id: label
>   text: "Hello"
> 
>   Button {
>     id: button
>     onClick: { label.hello = "World"; }
>   }
> }
> 
> "button" is an object that is declared inside label, and thus has an inner context that is a child of the context of "label". However, "button" cannot see the "text" property of "label", even if this property is declared in a parent context. "button" can see "label", though, as "label" is declared in the top-level context (using Declaration::setContext, this was already the case without this patch).
> 
> The inner context of a QML object therefore must be able to see the top-level context, its imported parent contexts (class inheritance), but not its parent context. This patch handles that by introducing QmlDUContext, a subclass of DUContext. QmlDUContext provides a method that allows DeclarationBuilder to specify that a context should be blind to its parent (but not the top-level context, hence the addImportedParentContext(topContext()) call).
> 
> 
> Diffs
> -----
> 
>   codecompletion/tests/qmlcompletiontest.cpp c029ea9 
>   duchain/CMakeLists.txt b02a257 
>   duchain/declarationbuilder.h 1e6c895 
>   duchain/declarationbuilder.cpp ede6407 
>   duchain/qmlducontext.h PRE-CREATION 
>   duchain/qmlducontext.cpp PRE-CREATION 
>   tests/files/test.qml cbe2fa6 
> 
> Diff: https://git.reviewboard.kde.org/r/118575/diff/
> 
> 
> Testing
> -------
> 
> Two new unit tests have been added. The first one ensures that the code-completion doesn't list symbols that should not be visible, and the second one checks that UseBuilder does not see uses where there shouldn't be any.
> 
> All the other unit tests still pass, but I'm not very confident of the implementation of QmlDUContext. There were many things to take into consideration, many constructors to implement, a macro that mustn't be forgotten, special setClassId calls, etc. I hope that I haven't missed anything, but I'm not sure (valgrind doesn't complain about anything, though, but I'm afraid of subtle cache-related bugs). No other language support plugin that I know (kdev-clang, kdev-oldcpp, kdev-python, kdev-ruby) subclasses DUContextData (they subclass DUContext, though), so I had no example.
> 
> 
> Thanks,
> 
> Denis Steckelmacher
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140606/db7c1a2a/attachment.html>