Status: New Owner: ---- Labels: Type-Defect Priority-Medium Component-Famix Milestone-4.4
New issue 606 by tudor.gi...@gmail.com: 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.
Updates: Labels: -Priority-Medium Priority-Critical
Comment #1 on issue 606 by tudor.gi...@gmail.com: FAMIXPackage>>methods is wrong http://code.google.com/p/moose-technology/issues/detail?id=606
(No comment was entered for this change.)
Comment #2 on issue 606 by jannik.l...@gmail.com: 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 ?
Updates: Cc: tudor.gi...@gmail.com jannik.l...@gmail.com
Comment #3 on issue 606 by jannik.l...@gmail.com: FAMIXPackage>>methods is wrong http://code.google.com/p/moose-technology/issues/detail?id=606
(No comment was entered for this change.)
Comment #4 on issue 606 by tudor.gi...@gmail.com: 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).
Comment #5 on issue 606 by jannik.l...@gmail.com: 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.
Comment #6 on issue 606 by tudor.gi...@gmail.com: 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.
Comment #7 on issue 606 by jannik.l...@gmail.com: 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 ?
Comment #8 on issue 606 by tudor.gi...@gmail.com: 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).
Comment #9 on issue 606 by jannik.l...@gmail.com: 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.
Comment #10 on issue 606 by tudor.gi...@gmail.com: 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.
Comment #11 on issue 606 by jannik.l...@gmail.com: 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 ?
Comment #12 on issue 606 by tudor.gi...@gmail.com: 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?
Comment #13 on issue 606 by jannik.l...@gmail.com: FAMIXPackage>>methods is wrong http://code.google.com/p/moose-technology/issues/detail?id=606
I am beginning to work on it.
Comment #14 on issue 606 by jannik.l...@gmail.com: 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.
Updates: Status: Fixed
Comment #15 on issue 606 by jannik.l...@gmail.com: FAMIXPackage>>methods is wrong http://code.google.com/p/moose-technology/issues/detail?id=606
I fixed them. All tests pass.
Updates: Status: Started
Comment #16 on issue 606 by jannik.l...@gmail.com: FAMIXPackage>>methods is wrong http://code.google.com/p/moose-technology/issues/detail?id=606
we should fix parentPackage. move this behavior in packageScope
Updates: Status: Fixed
Comment #17 on issue 606 by jannik.l...@gmail.com: 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