Skip to content

Composite Extract Method fails when replacing code with non-local return #18403

@balsa-sarenac

Description

@balsa-sarenac

Tested in Pharo 13 not in Pharo 14.

In this method:

SoupParser >> parseComment: anInteger
	| matcher |
	matcher := '--\s*>' asRegex.
	(matcher searchStream: (self readStreamFrom: anInteger))
		ifTrue: [
			self handleComment: (string 
				copyFrom: anInteger + 5 
				to: (matcher subBeginning: 1) first).
			^( matcher subEnd: 1) first.
		].
	^ self parseIncomplete: anInteger

when extracting:

self handleComment: (string 
				copyFrom: anInteger + 5 
				to: (matcher subBeginning: 1) first).
			^( matcher subEnd: 1) first.

I get a failure that it cannot add nodes after return statement.
This is because it is creating parse tree rewrite template for replacement and tries to add more nodes @.S2 after return node.

Proposed fix:

Update the check here (before invoking addNode:) that will check if tree already has return node as the last node in the statements list. (This is method inside ReplaceSubtreeTransformation class)

(tree statements last = aParseTree body statements last)
		ifFalse:
			[tree addNode: (OCPatternVariableNode named: '`@.S2').
			replaceStmt := replaceStmt, '. `@.S2'].

to:

	tree statements last isReturn ifFalse: [ 
	(tree statements last = aParseTree body statements last)
		ifFalse:
			[tree addNode: (OCPatternVariableNode named: '`@.S2').
			replaceStmt := replaceStmt, '. `@.S2'].
		 ].

Some pictures of the bug:

Image Image Image

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    Status

    For Later (kind of Backlog)

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions