Bug 336810

Created attachment 87437 [details]
c++ header resulting into the wrong base class assignment

The problem is that in Import_Utils::createGeneralization() 

void createGeneralization(UMLClassifier *child, const QString &parentName)
{
    UMLObject *parentObj = createUMLObject(UMLObject::ot_Class, parentName);
    UMLClassifier *parent = static_cast<UMLClassifier*>(parentObj);
    createGeneralization(child, parent);
}

createUMLObject() is called with the global namespace as parent (third parameter=0), which search (and creates it not found) the related class in the global namespace, while it is required to find the namespace of the parent class first and to provide it to createUMLObject().

Git commit 0160b7d8f8f76f3c2da00012314f0e4454c7105e by Oliver Kellogg.
Committed on 28/06/2014 at 08:23.
Pushed by okellogg into branch 'master'.

umbrello/codeimport/kdevcppparser/cpptree2uml.cpp
- In function parseBaseClause() first call Import_Utils::createUMLObject()
  for creating the parent object coresponding to baseName, and then call
    Import_Utils::createGeneralization(UMLClassifier*, UMLClassifier*)
  with the parent object obtained.

umbrello/codeimport/import_utils.cpp
- At function
    createGeneralization(UMLClassifier *child, const QString &parentName)
  mention its shortcoming and mention the preferred solution as applied
  above.

M  +4    -0    umbrello/codeimport/import_utils.cpp
M  +5    -1    umbrello/codeimport/kdevcppparser/cpptree2uml.cpp

http://commits.kde.org/umbrello/0160b7d8f8f76f3c2da00012314f0e4454c7105e

(In reply to comment #3)
> Git commit 0160b7d8f8f76f3c2da00012314f0e4454c7105e by Oliver Kellogg.
> Committed on 28/06/2014 at 08:23.
> Pushed by okellogg into branch 'master'.
> 
> umbrello/codeimport/kdevcppparser/cpptree2uml.cpp
> - In function parseBaseClause() first call Import_Utils::createUMLObject()
>   for creating the parent object coresponding to baseName, and then call
>     Import_Utils::createGeneralization(UMLClassifier*, UMLClassifier*)
>   with the parent object obtained.
This fixes the mentioned case, thanks. Unfortunally the fix seems not to be complete because with the following testcase class GlobalA is shown in Logical View but assigned to namespace NameSpace which is wrong.

class GlobalA {
public:  
  GlobalA();
};

namespace NameSpace {

class NameSpacedC : public GlobalA
{
public:
  NameSpacedC();
};
}

The following testcase fails too
/// class Global::A
class A { public:  A(); };

namespace N1 {

  /// class N1::A
class A { public:  A(); };
}

namespace N2 {

/// class N2::A
class A { public:  A(); };

/// class N2::B
class B : public A { public: B(); };

/// class N2::C
class C : public ::A { public; C(); };

/// class N2::D
class C : public N1::A { public; C(); };
}

An additional failing testcase

/// class Global::A
class A { public:  A(); };

namespace N1 {

  /// class N1::A
class A { public:  A(); };
}

namespace N2 {

  /// class N2::A
class A { public:  A(); };
}

using N2;

namespace N3 {

/// class N3::A
class A { public:  A(); };

/// class N3::B
class B : public A { public: B(); };

/// class N3::C
class C : public ::A { public; C(); };

/// class N3::D
class C : public N1::A { public; C(); };

/// class N3::E
class D : public N2::A { public; C(); };
}

Git commit f90f771bc4ee756d7e9e3e53794d889ef802e45f by Oliver Kellogg.
Committed on 28/06/2014 at 18:38.
Pushed by okellogg into branch 'master'.

(In reply to comment #4)
> [...] Unfortunately the fix seems not to be complete because with
> the following testcase class GlobalA is shown in Logical View but
> assigned to namespace NameSpace which is wrong.

Here is the fix for this case.
The fixing of your other test cases is still wide open, help appreciated.

umbrello/codeimport/kdevcppparser/cpptree2uml.cpp
- In function parseBaseClause(), before calling
  Import_Utils::createUMLObject() for creating the parent object, call
  Import_Utils::putAtGlobalScope(true). After creation of parent object,
  call Import_Utils::putAtGlobalScope(false).

M  +2    -0    umbrello/codeimport/kdevcppparser/cpptree2uml.cpp

http://commits.kde.org/umbrello/f90f771bc4ee756d7e9e3e53794d889ef802e45f

(In reply to comment #6)
> Git commit f90f771bc4ee756d7e9e3e53794d889ef802e45f by Oliver Kellogg.
> Committed on 28/06/2014 at 18:38.
> Pushed by okellogg into branch 'master'.
> 
> (In reply to comment #4)
> > [...] Unfortunately the fix seems not to be complete because with
> > the following testcase class GlobalA is shown in Logical View but
> > assigned to namespace NameSpace which is wrong.
> 
> Here is the fix for this case.
works, thanks 

> The fixing of your other test cases is still wide open, help appreciated.

Because createUMLObject already has a basic object find strategy inside I assume that we need to split/refactor createUMLObject to get better control where and what to search ?

(In reply to comment #5)
> An additional failing testcase
> 
...

> namespace N1 {
> 
>   /// class N1::A
> class A { public:  A(); };
> }
> 
The problem here is that N1::A shows package path N1 in properties (which is correct), but is shown directly below logical view in the tree view and also could not moved into N1 namespace

(In reply to comment #8)
> (In reply to comment #5)
> > An additional failing testcase
> > 
> ...
> 
> > namespace N1 {
> > 
> >   /// class N1::A
> > class A { public:  A(); };
> > }
> > 
> The problem here is that N1::A shows package path N1 in properties (which is
> correct), but is shown directly below logical view in the tree view
In detail: On calling import_utils.cpp::CreateUmlObject() to create N1::A  the call to 
UMLObject * o = umldoc->findUMLObject(name, type, parentPkg);

on line import_utils.cpp:209 returns the UMLObject of class A  in the global namespace which is wrong; it should return zero. 

The reason why the global object is returned is documented in Model_Utils::findUMLObject()

 * @param currentObj    Object relative to which to search (optional.)
 *                      If given then the enclosing scope(s) of this
 *                      object are searched before the global scope.

The search should be limit to the namespace given by currentObj.

Created attachment 87497 [details]
Simplified namespace test case mentioned in comment 5

Remaining tree view issue is that class N1:A is not located in the N1 package.

Git commit b37eebda3b4b0387486fe80d0e329a73c8f6de72 by Ralf Habacker.
Committed on 01/07/2014 at 14:08.
Pushed by habacker into branch 'master'.

Fix bug not placing class N1::A below N1 package in tree view.

The problem was, that Import_Utils::createUMLObject() tries to find an object
in different namespaces and only creates an uml object if not found.

In the case of parsing a c++ import class specification we simply need
to create the related uml class object in the current namespace, which
is provided by the additional parameter "doNotSearch".

Related test case is https://bugs.kde.org/show_bug.cgi?id=336810#c10

M  +11   -3    umbrello/codeimport/import_utils.cpp
M  +2    -1    umbrello/codeimport/import_utils.h
M  +1    -1    umbrello/codeimport/kdevcppparser/cpptree2uml.cpp
M  +4    -0    umbrello/object_factory.h

http://commits.kde.org/umbrello/b37eebda3b4b0387486fe80d0e329a73c8f6de72

Git commit 76fa95164959012384fca4c524dcbba548abc496 by Ralf Habacker.
Committed on 01/07/2014 at 20:07.
Pushed by habacker into branch 'master'.

Move creating of uml artifact objects into separate method Import_Utils::createArtifact().

M  +26   -15   umbrello/codeimport/import_utils.cpp
M  +4    -0    umbrello/codeimport/import_utils.h
M  +1    -1    umbrello/codeimport/kdevcppparser/cpptree2uml.cpp

http://commits.kde.org/umbrello/76fa95164959012384fca4c524dcbba548abc496

*** Bug 193080 has been marked as a duplicate of this bug. ***