Issue 606 in moose-technology: FAMIXPackage>>methods is wrong

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium Component-Famix Milestone-4.4

New issue 606 by [hidden email]: FAMIXPackage>>methods is wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

in FAMIXPackage, methods only returns the extended methods defined in the  
package:

FAMIXPackage>>methods
        ^ self privateState
                        cacheAt: #methods
                        ifAbsentPut: [
                                childNamedEntities select: [ :child | child isKindOf:  
FAMIXBehaviouralEntity ]]

This has a different semantics from the one defined in Namespace:

FAMIXNamespace>>methods
        ^ self privateState
                        cacheAt: #methods
                        ifAbsentPut: [
                                self classes flatCollect: #methods ]

So. I propose that FAMIXPackage>>methods is changed to extensionMethods and  
methods is computed like in Namespace + extensionMethods.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Updates:
        Labels: -Priority-Medium Priority-Critical

Comment #1 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

(No comment was entered for this change.)

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #2 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

extensionMethods is already defined as
"self privateState cacheAt: #extensionMethods ifAbsentPut: [ self methods  
select: [ :m| m isExtension ] ]"

What I understand is to have:
- extensionMethods without changes of the source code
- localMethods that we should modify to have the same as Namespace
- methods that is a union of extensionMethods and localMethods

Am I right ?

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Updates:
        Cc: [hidden email] [hidden email]

Comment #3 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

(No comment was entered for this change.)

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #4 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

Pretty much, only that localMethods should be definedMethods (it already  
exists, only with the wrong implementation).

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #5 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

Not sure,
I prefer localMethod than definedMethod for non-extension methods.

For now definedMethod is an alias of methods.
We should pay attention to not break the semantic already used.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #6 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

Right now, the semantics of methods is to provide the extension methods  
only. So, currently definedMethods returns only the extension methods which  
is wrong.

Given that the famix-extension API currently depends on this, it will raise  
erroneous results, so we will have to revisit it anyway. We will probably  
rewrite it in terms of Chef.

In any case, perhaps localMethods is better for the moment. We should not  
depend on this method too much anyway.


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #7 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

I am not sure to understand: "Right now, the semantics of methods is to  
provide the extension methods only".

childNamedEntities contains all entities in the package, right ?
if we select "child isKindOf: FAMIXBehaviouralEntity", we select all  
methods, no ?

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #8 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

childNamedEntities contains all the directly packagedIn entities, so it  
will contain the defined classes and the extension methods. The methods  
defined in classes will be accessible from the classes objects, but they  
won't be stored directly in package (just like in namespace).

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #9 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

hum, that is the point of the discussion :)

When I browse a mooseModel, childNamedEntities contains all methods of the  
packages. And we do not need to access the class to obtain them.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #10 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

Hmm, I got confused by some other bug in the importer.

Please try the following:
- Import Famix-Core
- Browse All Packages
- You will find 2 packages: The Famix-Core package only contains the  
extensions, while the Default one contains all the rest of the classes

This is a bug, but I do not know what the source is. Maybe it's RPackage,  
but I do not have time to investigate.

In any case, coming back to the current issue, you are right that currently  
we specify the package of all methods. I think this is against the original  
intention. For example, we do not specify the packaging of attributes. So,  
I think we should only provide the package information when it cannot be  
deduced from a (belongsTo) container.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #11 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

I have not this bug. I use the Moose from Hudson (Apr 30, 2011 9:39:29 PM).
When I load Famix-Core, I have two packages: Famix-Core and  
Famix-Core-MooseChef.
Probably your RPackage is broken.

You are right. We should change that.
So, I summarize what we have to do:

- do not store package for a method that is contained in the same package  
of its class. That should be done directly in importers.
- extensionMethods : return methods contained in a package
- localMethods/definedMethods: returns methods contains in classes from the  
selected package
- methods that is a union of extensionMethods and localMethods

Now, what is the impact on all our tools ?
Is only SmalltalkImporter concerned by this change ?


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #12 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

Nice summary :).

I do not think anything else needs to be updated. Except for the API, but  
that needs to be reviewed anyway.

Do you give it a try, or should I?


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #13 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

I am beginning to work on it.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Comment #14 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

I have to redefine parentPackage in FAMIXMethod like that:

===
parentPackage

        ^ parentPackage ifNil:[ self belongsTo parentPackage]
===

Is it ok ?
If yes, the source code is ready to be pushed in repo.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Updates:
        Status: Fixed

Comment #15 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

I fixed them.
All tests pass.

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Updates:
        Status: Started

Comment #16 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

we should fix parentPackage.
move this behavior in packageScope

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev

Updates:
        Status: Fixed

Comment #17 on issue 606 by [hidden email]: FAMIXPackage>>methods is  
wrong
http://code.google.com/p/moose-technology/issues/detail?id=606

the code is now in packageScope.
And I change a lot of call to parentPackage in packageScope

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev