Skip to content

RBInlineMethodRefacotring has behavior preserving preconditions checked during execution #18154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
balsa-sarenac opened this issue May 12, 2025 · 0 comments

Comments

@balsa-sarenac
Copy link
Member

Right now Inline method refactoring has warning that pops-up during execution (transformation) phase. It should be moved to behavior preserving preconditions. It is here:

"transforming"
inlineSourceReplacing: aParseTree
	| statements nodeUnderSequence |
	statements := inlineParseTree body statements.
	(statements size > 1 and: [ aParseTree isEvaluatedFirst not ]) ifTrue: [
		self refactoringWarning: "<<---- THIS ONE"
			'To inline this method, we need to move some of its statements before the original message send.<n>This could change the order of execution, which can change the behavior.<n>Do you want to proceed?'
				expandMacros ].
	nodeUnderSequence := aParseTree.
	[ nodeUnderSequence parent isSequence ] whileFalse: [ nodeUnderSequence := nodeUnderSequence parent ].
	nodeUnderSequence parent
		addNodes: (statements copyFrom: 1 to: (statements size - 1 max: 0)) before: nodeUnderSequence;
		addTemporariesNamed: inlineParseTree body temporaryNames.
	aParseTree parent replaceNode: aParseTree withNode: (statements isEmpty
			 ifTrue: [ OCVariableNode selfNode ]
			 ifFalse: [ statements last ])

proposed fix:

"preconditions"
RBInlineMethodRefactoring >> breakingChangePreconditions

	^ { (RBCondition withBlock: [
		   self checkOverridden ifTrue: [
			   self isOverridden ifTrue: [
				   self refactoringWarning:
					   ('<1p>>><2s> is overriden. Do you want to inline it anyway?'
					    , String cr , 'You can break you hooks with this inline.'
						    expandMacrosWith: self classOfTheMethodToInline
						    with: self inlineSelector) ] ].
		   true ]).
		(RBCondition withBlock: [
			| statements |
			statements := inlineParseTree body statements.
			(statements size > 1 and: [ sourceMessage isEvaluatedFirst not ]) ifTrue: [
				self refactoringWarning:
					'To inline this method, we need to move some of its statements before the original message send.<n>This could change the order of execution, which can change the behavior.<n>Do you want to proceed?'
						expandMacros 
			]]) 
	}
RBInlineMethodRefactoring >> inlineSourceReplacing: aParseTree
	| statements nodeUnderSequence |
	statements := inlineParseTree body statements.
	nodeUnderSequence := aParseTree.
	[ nodeUnderSequence parent isSequence ] whileFalse: [ nodeUnderSequence := nodeUnderSequence parent ].
	nodeUnderSequence parent
		addNodes: (statements copyFrom: 1 to: (statements size - 1 max: 0)) before: nodeUnderSequence;
		addTemporariesNamed: inlineParseTree body temporaryNames.
	aParseTree parent replaceNode: aParseTree withNode: (statements isEmpty
			 ifTrue: [ OCVariableNode selfNode ]
			 ifFalse: [ statements last ])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: For Later (kind of Backlog)
Development

No branches or pull requests

1 participant