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

[Sven Brauch]

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


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?


duchain/qmlducontext.h
<https://git.reviewboard.kde.org/r/118575/#comment41312>

    maybe that docstring fits better with the method than with the class



duchain/qmlducontext.h
<https://git.reviewboard.kde.org/r/118575/#comment41313>

    this function needs a docstring, mentioning it's overriden and for what reason ;)



duchain/qmlducontext.cpp
<https://git.reviewboard.kde.org/r/118575/#comment41314>

    what's this for? maybe you can put a comment?


- Sven Brauch


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/20140605/b98b784e/attachment.html>