pharo icon indicating copy to clipboard operation
pharo copied to clipboard

Fix for Negated condition violators (retry)

Open carolahp opened this issue 4 months ago • 4 comments

carolahp avatar Oct 23 '25 12:10 carolahp

This issue has either a default title or empty body. We would appreciate it if you could provide more information. Note: I am not a very intelligent bot, I can only react to new comments. Please add a comment for me if you update the body or title.

request-info[bot] avatar Oct 23 '25 12:10 request-info[bot]

What is the status of this PR? Should I do a full review?

I am about to finish, I only have 13 tests not passing. I merged the changes from PR#18890, to work on the latest version, in a fresh image. I will merge it all back to this branch as soon as I fix the tests

carolahp avatar Nov 13 '25 12:11 carolahp

There is a failing test

testFailure
Found undeclared Variables: 
RBProtectVariableTransformation in: RBProtectVariableTransformationTest>>#testVariableDoesNotExist
ReleaseTest(TestAsserter)>>assert:description:resumable:
ReleaseTest(TestAsserter)>>assert:description:
[ "we compile a second method with the undeclared #undeclaredStubInstVar1 to trigger the code path of removing twice in #cleanOutUndeclared"
	self class compile: 'methodForTest ^undeclaredStubInstVar1'.
	Smalltalk cleanOutUndeclared.
	undeclaredVariables := Undeclared associations select: [ :each | each isUndeclaredVariable ].

	validExceptions := { #undeclaredStubInstVar1. #undeclaredStubInstVar2. #Gofer }. "The laste one is for Smalltalk CI and our external projects... But we should find a better solution at some point."

	"for now we filter by name, maybe filtering by variable would be better"
	remaining := undeclaredVariables reject: [ :each | validExceptions includes: each name ].

	"we look for one of the using methods of the undeclared var and report that,
	this should be enough to fix it quickly"
	description := String streamContents: [ :stream |
		               stream nextPutAll: 'Found undeclared Variables: '.
		               remaining do: [ :variable |
			               | method |
			               method := variable usingMethods anyOne.
			               stream cr
				               nextPutAll: variable name;
				               nextPutAll: ' in: ';
				               print: method methodClass;
				               nextPutAll: '>>';
				               print: method selector ] ].

	self assert: remaining isEmpty description: description ] in ReleaseTest>>testUndeclared
FullBlockClosure(BlockClosure)>>ensure:
ReleaseTest>>testUndeclared
ReleaseTest(TestCase)>>performTest
		

Ducasse avatar Nov 15 '25 20:11 Ducasse

We should merge P14 in this branch because I fixed those tests 2 days ago I think

jecisc avatar Nov 15 '25 20:11 jecisc