jecisc / Bazard

0 stars 0 forks source link

RBCascadeNextPutAllsRule refactoring crash BlueInk #129

Closed jecisc closed 5 years ago

jecisc commented 5 years ago

Migrated case from Manuscript.

Original case: https://pharo.fogbugz.com/f/cases/22250 Status: Work Needed Project: SmallLint Original Author: CyrilFerlicot Date: 5 July 2018 1:15:39 pm

Description:

I have this method:

storeOn: aStream aStream nextPutAll: '(' , self class name; nextPutAll: ' tint: '; print: (self tint); nextPutAll: ' code: '; print: (self code); nextPutAll: ')'.

The RBCascadeNextPutAllsRule propose an automatic refactoring but when I try to use it I get an error.

Instance of RBCascadeNode did not understand #selector

RBCascadeNode(Object)>>doesNotUnderstand: #selector BIConfigurableFormatter>>isMultiLineMessage: [ self formatCommentsFor: each. self formatSelectorAndArguments: each firstSeparator: [ ] restSeparator: ((self isMultiLineMessage: each) ifTrue: [ [ self newLine ] ] ifFalse: [ [ self space ] ]) ] in [ :each | self indentAround: [ self formatCommentsFor: each. self formatSelectorAndArguments: each firstSeparator: [ ] restSeparator: ((self isMultiLineMessage: each) ifTrue: [ [ self newLine ] ] ifFalse: [ [ self space ] ]) ] ] in [ self newLineBeforeFirstCascade ifTrue: [ self newLine ] ifFalse: [ self space ]. aCascadeNode messages do: [ :each | self indentAround: [ self formatCommentsFor: each. self formatSelectorAndArguments: each firstSeparator: [ ] restSeparator: ((self isMultiLineMessage: each) ifTrue: [ [ self newLine ] ] ifFalse: [ [ self space ] ]) ] ] separatedBy: [ codeStream nextPut: $;. self newLineAfterCascade ifTrue: [ self newLine ] ifFalse: [ self space ] ] ] in BIConfigurableFormatter>>visitCascadeNode: in Block: [ self formatCommentsFor: each.... BlockClosure>>ensure: BIConfigurableFormatter>>indent:around: BIConfigurableFormatter>>indentAround: [ :each | self indentAround: [ self formatCommentsFor: each. self formatSelectorAndArguments: each firstSeparator: [ ] restSeparator: ((self isMultiLineMessage: each) ifTrue: [ [ self newLine ] ] ifFalse: [ [ self space ] ]) ] ] in [ self newLineBeforeFirstCascade ifTrue: [ self newLine ] ifFalse: [ self space ]. aCascadeNode messages do: [ :each | self indentAround: [ self formatCommentsFor: each. self formatSelectorAndArguments: each firstSeparator: [ ] restSeparator: ((self isMultiLineMessage: each) ifTrue: [ [ self newLine ] ] ifFalse: [ [ self space ] ]) ] ] separatedBy: [ codeStream nextPut: $;. self newLineAfterCascade ifTrue: [ self newLine ] ifFalse: [ self space ] ] ] in BIConfigurableFormatter>>visitCascadeNode: in Block: [ :each | ... OrderedCollection(SequenceableCollection)>>do:separatedBy: [ self newLineBeforeFirstCascade ifTrue: [ self newLine ] ifFalse: [ self space ]. aCascadeNode messages do: [ :each | self indentAround: [ self formatCommentsFor: each. self formatSelectorAndArguments: each firstSeparator: [ ] restSeparator: ((self isMultiLineMessage: each) ifTrue: [ [ self newLine ] ] ifFalse: [ [ self space ] ]) ] ] separatedBy: [ codeStream nextPut: $;. self newLineAfterCascade ifTrue: [ self newLine ] ifFalse: [ self space ] ] ] in BIConfigurableFormatter>>visitCascadeNode: in Block: [ self newLineBeforeFirstCascade... BlockClosure>>ensure: BIConfigurableFormatter>>indent:around: BIConfigurableFormatter>>indentAround: BIConfigurableFormatter>>visitCascadeNode: RBCascadeNode>>acceptVisitor: BIConfigurableFormatter(RBProgramNodeVisitor)>>visitNode: [ needsParenthesis ifTrue: [ codeStream nextPutAll: self stringInsideParentheses ]. super visitNode: aNode. (self formatCommentWithStatements or: [ aNode isMethod or: [ aNode isSequence ] ]) ifFalse: [ self formatCommentsFor: aNode ]. needsParenthesis ifTrue: [ codeStream nextPutAll: self stringInsideParentheses ] ] in BIConfigurableFormatter>>visitNode: in Block: [ needsParenthesis... BIConfigurableFormatter>>bracketWith:around: BIConfigurableFormatter>>visitNode: BIConfigurableFormatter>>formatSequenceNodeStatementsFor: BIConfigurableFormatter>>visitSequenceNode: RBSequenceNode>>acceptVisitor: BIConfigurableFormatter(RBProgramNodeVisitor)>>visitNode: [ needsParenthesis ifTrue: [ codeStream nextPutAll: self stringInsideParentheses ]. super visitNode: aNode. (self formatCommentWithStatements or: [ aNode isMethod or: [ aNode isSequence ] ]) ifFalse: [ self formatCommentsFor: aNode ]. needsParenthesis ifTrue: [ codeStream nextPutAll: self stringInsideParentheses ] ] in BIConfigurableFormatter>>visitNode: in Block: [ needsParenthesis... BIConfigurableFormatter>>bracketWith:around: BIConfigurableFormatter>>visitNode: [ self newLines: self newLinesAfterMethodPattern. self formatMethodCommentFor: aMethodNode. self formatPragmasFor: aMethodNode. self visitNode: aMethodNode body ] in BIConfigurableFormatter>>formatMethodBodyFor: in Block: [ self newLines: self newLinesAfterMethodPattern.... BlockClosure>>ensure: BIConfigurableFormatter>>indent:around: BIConfigurableFormatter>>indentAround:

jecisc commented 5 years ago

Reply: Author: Pavel Krivanek Date: 28 September 2018 12:28:39 pm

Message:

New in Pharo 7

jecisc commented 5 years ago

Reply: Author: CyrilFerlicot Date: 5 October 2018 8:38:08 am

Message:

Apparently, before QA used ReTransformationCritique and now it uses ReReplaceNodeCritique.

The new one produce a wrong AST for this transformation.

jecisc commented 5 years ago

Reply: Author: CyrilFerlicot Date: 10 January 2019 3:02:34 pm

Message:

In Pharo 6 this critic did a rewrite of the method using RBParseTreeRewriter. This one managed the case where the refactored node was contained in a cascade or not.

In Pharo 7 the rewrite is done in ReReplaceNodeCritique. But this one uses a becomeForward: to replace an old node by a rewritten node. But this breaks in case the new node cannot just replace the old one, like when it tries to introduce a cascade in another cascade.

I don't know renraku enough to fix the problem and I was wondering why ReReplaceNodeCritique does the replacement itself when we already have RBParseTreeNode?

Anyway, it's probably too complex to fix for Pharo 7. Moving to Pharo 8.