[Pharo-project] interesting source code inconsistency
Hi, the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method. http://code.google.com/p/pharo/issues/detail?id=5463 Cheers, -- Pavel
thanks pavel. One of these days we will have to improve that part too. Stef On Mar 11, 2012, at 6:43 PM, Pavel Krivanek wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
Hi Pavel, I presume they are found by the following: SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]] This could be a "browse unbound", and could be included in release tests via e.g. SystemNavigation>methodsWithUnboundGlobals ^self allSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]] (or whereever Pharo puts browsing queries these days) On Sun, Mar 11, 2012 at 10:43 AM, Pavel Krivanek <pavel.krivanek@gmail.com>wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
-- best, Eliot
On Sun, Mar 11, 2012 at 3:33 PM, Eliot Miranda <eliot.miranda@gmail.com>wrote:
Hi Pavel,
I presume they are found by the following:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
This could be a "browse unbound", and could be included in release tests via e.g.
SystemNavigation>methodsWithUnboundGlobals ^self allSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
(or whereever Pharo puts browsing queries these days)
and I suppose it should exclude Undeclared variables, so this is better: SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil and: [(Undeclared includesAssociation: l) not]]]]]
On Sun, Mar 11, 2012 at 10:43 AM, Pavel Krivanek <pavel.krivanek@gmail.com
wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
-- best, Eliot
-- best, Eliot
Thank you, Eliot. I updated the issue report. -- Pavel On Sun, Mar 11, 2012 at 11:36 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
On Sun, Mar 11, 2012 at 3:33 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Pavel,
  I presume they are found by the following:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
This could be a "browse unbound", and could be included in release tests via e.g.
SystemNavigation>methodsWithUnboundGlobals ^self allSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
(or whereever Pharo puts browsing queries these days)
and I suppose it should exclude Undeclared variables, so this is better:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil and: [(Undeclared includesAssociation: l) not]]]]]
On Sun, Mar 11, 2012 at 10:43 AM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
-- best, Eliot
-- best, Eliot
Hi Pavel, On Mon, Mar 12, 2012 at 1:40 AM, Pavel Krivanek <pavel.krivanek@gmail.com>wrote:
Thank you, Eliot. I updated the issue report.
I'm grabbing the changes for Squeak trunk. But I don't see why one should do cleanOutUndeclared in testMethodsWithUnboundGlobals. It doesn't affect the result of the test (since methodsWittUnboundGlobals filters-out Undeclared) and introduces a side-effect of running tests.
-- Pavel
On Sun, Mar 11, 2012 at 11:36 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
On Sun, Mar 11, 2012 at 3:33 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Pavel,
I presume they are found by the following:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
This could be a "browse unbound", and could be included in release tests via e.g.
SystemNavigation>methodsWithUnboundGlobals ^self allSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
(or whereever Pharo puts browsing queries these days)
and I suppose it should exclude Undeclared variables, so this is better:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil and: [(Undeclared includesAssociation: l) not]]]]]
On Sun, Mar 11, 2012 at 10:43 AM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
-- best, Eliot
-- best, Eliot
-- best, Eliot
Hi, I followed content of testUndeclared so side effect of ReleaseTest was already there. And because we filter out Undeclared, I wanted to have in Undeclared the valid content. BTW looking at the code again, maybe we should not test includesAssociation: but directly identity of associations. -- Pavel On Mon, Mar 12, 2012 at 9:47 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Pavel,
On Mon, Mar 12, 2012 at 1:40 AM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Thank you, Eliot. I updated the issue report.
I'm grabbing the changes for Squeak trunk.  But I don't see why one should do cleanOutUndeclared in testMethodsWithUnboundGlobals.  It doesn't affect the result of the test (since methodsWittUnboundGlobals filters-out Undeclared) and introduces a side-effect of running tests.
-- Pavel
On Sun, Mar 11, 2012 at 11:36 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
On Sun, Mar 11, 2012 at 3:33 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Pavel,
  I presume they are found by the following:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
This could be a "browse unbound", and could be included in release tests via e.g.
SystemNavigation>methodsWithUnboundGlobals ^self allSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
(or whereever Pharo puts browsing queries these days)
and I suppose it should exclude Undeclared variables, so this is better:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil and: [(Undeclared includesAssociation: l) not]]]]]
On Sun, Mar 11, 2012 at 10:43 AM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
-- best, Eliot
-- best, Eliot
-- best, Eliot
Thanks, Pavel, you're quite right. I've committed code to Squeak trunk. Forgive the reformat ;) On Mon, Mar 12, 2012 at 2:14 PM, Pavel Krivanek <pavel.krivanek@gmail.com>wrote:
Hi,
I followed content of testUndeclared so side effect of ReleaseTest was already there. And because we filter out Undeclared, I wanted to have in Undeclared the valid content. BTW looking at the code again, maybe we should not test includesAssociation: but directly identity of associations.
-- Pavel
On Mon, Mar 12, 2012 at 9:47 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Pavel,
On Mon, Mar 12, 2012 at 1:40 AM, Pavel Krivanek < pavel.krivanek@gmail.com> wrote:
Thank you, Eliot. I updated the issue report.
I'm grabbing the changes for Squeak trunk. But I don't see why one should do cleanOutUndeclared in testMethodsWithUnboundGlobals. It doesn't affect the result of the test (since methodsWittUnboundGlobals filters-out Undeclared) and introduces a side-effect of running tests.
-- Pavel
On Sun, Mar 11, 2012 at 11:36 PM, Eliot Miranda <
eliot.miranda@gmail.com>
wrote:
On Sun, Mar 11, 2012 at 3:33 PM, Eliot Miranda <
eliot.miranda@gmail.com>
wrote:
Hi Pavel,
I presume they are found by the following:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
This could be a "browse unbound", and could be included in release tests via e.g.
SystemNavigation>methodsWithUnboundGlobals ^self allSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
(or whereever Pharo puts browsing queries these days)
and I suppose it should exclude Undeclared variables, so this is better:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil and: [(Undeclared includesAssociation: l) not]]]]]
On Sun, Mar 11, 2012 at 10:43 AM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency
is
not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
-- best, Eliot
-- best, Eliot
-- best, Eliot
-- best, Eliot
Isn't Smalltalk With Style source of the right formating? ;-) -- Pavel On Mon, Mar 12, 2012 at 10:31 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Thanks, Pavel, you're quite right. Â I've committed code to Squeak trunk. Â Forgive the reformat ;)
On Mon, Mar 12, 2012 at 2:14 PM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Hi,
I followed content of testUndeclared so side effect of ReleaseTest was already there. And because we filter out Undeclared, I wanted to have in Undeclared the valid content. BTW looking at the code again, maybe we should not test includesAssociation: but directly identity of associations.
-- Pavel
On Mon, Mar 12, 2012 at 9:47 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Pavel,
On Mon, Mar 12, 2012 at 1:40 AM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Thank you, Eliot. I updated the issue report.
I'm grabbing the changes for Squeak trunk.  But I don't see why one should do cleanOutUndeclared in testMethodsWithUnboundGlobals.  It doesn't affect the result of the test (since methodsWittUnboundGlobals filters-out Undeclared) and introduces a side-effect of running tests.
-- Pavel
On Sun, Mar 11, 2012 at 11:36 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
On Sun, Mar 11, 2012 at 3:33 PM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Hi Pavel,
  I presume they are found by the following:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
This could be a "browse unbound", and could be included in release tests via e.g.
SystemNavigation>methodsWithUnboundGlobals ^self allSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil]]]]
(or whereever Pharo puts browsing queries these days)
and I suppose it should exclude Undeclared variables, so this is better:
SystemNavigation new browseAllSelect: [:m| m literals anySatisfy: [:l| l isVariableBinding and: [l key isSymbol "avoid class-side methodClass literals" and: [(m methodClass bindingOf: l key) isNil and: [(Undeclared includesAssociation: l) not]]]]]
On Sun, Mar 11, 2012 at 10:43 AM, Pavel Krivanek <pavel.krivanek@gmail.com> wrote:
Hi,
the method DefaultSettingStyle>>#load includes several obsolete classes (RBProgramNode, RBConfigurableFormatter, RefactoryChangeManager). It is interesting that this inconsistency is not reported by Undeclared nor obsoleteClasses (so release tests are green). The literals array contains associations to nil pointed only from the literal array of this method.
http://code.google.com/p/pharo/issues/detail?id=5463
Cheers, -- Pavel
-- best, Eliot
-- best, Eliot
-- best, Eliot
-- best, Eliot
participants (3)
-
Eliot Miranda -
Pavel Krivanek -
Stéphane Ducasse