String as integer

[jordanmontt]

This PR Does the following:

• Changed the semantics of the method String>>#asInteger Now, it returns a integer ONLY if the complete String is a valid integer, else it raises an exception.
• Deprecated String>>#asSignedInteger.
• Copied the old implementation of String>>#asInteger to String>>#extractIntegerPart. That new method behaves exactly as the old asInteger.
• Changed the semantics of String>>#asUnsignedInteger to just call String>>#asInteger and then getting the absolute value.
• Updated all senders of String>>#asInteger to call String>>#extractIntegerPart instead.
• Updated and added test to document this behevior.

This also closes #17277

[MarcusDenker]

Full image load fails with:

MetacelloNotification: Loaded -> Calypso-SystemPlugins-Monticello-Browser --- tonel:///builds/workspace/est_and_branch_Pipeline_PR-17299/64/src()
MetacelloNotification: Loaded -> Deprecated13 --- tonel:///builds/workspace/est_and_branch_Pipeline_PR-17299/64/src()
Removing credential.Error: The string '1. 338 forks on GitHub' is not a valid integer

which looks related

[jordanmontt]

Yes, the error is related. It is caused by Microdown. There is already a PR that will fix it: pillar-markup/Microdown#901

But, the Microdown PR requires this PR to be merged first. So, We will have the failing tests for a build, then we merge the Microdown PR and all the tests should return to be green.

[jordanmontt]

Thanks for the suggestion @astares I added the test case

[Ducasse]

I have no idea how to fix these tests. Probably somewhere in the baseline the microdown inside Pharo should be unloaded
but I have no idea where it is.

I will propose to remove Microdown from Pharo because it just slows me down all the time.
For the record all microdown tests are green and I'm systematically adding more of them.

#########################
# 2 tests did not pass: #
#########################

ReleaseTest
 ✗ #testUndeclared (151ms)
CollectionIsEmpty: a WeakIdentitySet() is empty
WeakIdentitySet(Collection)>>errorEmptyCollection
WeakIdentitySet(Collection)>>emptyCheck
WeakIdentitySet(Collection)>>anyOne
WeakIdentitySet(WeakSet)>>anyOne
[ :variable |
			               | method |
			               method := variable usingMethods anyOne.
			               stream cr
				               nextPutAll: variable name;
				               nextPutAll: ' in: ';
				               print: method methodClass;
				               nextPutAll: '>>';
				               print: method selector ] in [ :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 ] ] in [ "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 ...anyOne
Array(SequenceableCollection)>>do:
[ :stream |
		               stream nextPutAll: 'Found undeclared Variables: '.
		               remaining do: [ :variable |
			               | method |
			               method := variable usingMethods anyOne.
			               stream cr
				               nextPutAll: variable name;
	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 ...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 ] ]
FullBlockClosure(BlockClosure)>>ensure:
ReleaseTest>>testUndeclared ...ensure: [ self class removeSelector: #methodForTest ]
ReleaseTest(TestCase)>>performTest
 ✗ #testObsoleteClasses (831ms)
TestFailure: Obsolete classes remaining: an Array(AnObsoleteMicResourceReference AnObsoleteMicAbsoluteResourceReference AnObsoleteMicHTTPResourceReference AnObsoleteMicPharoImageResourceReference AnObsoleteMicrodownVisitor AnObsoleteMicArgumentList AnObsoleteMicMicrodownSharedPool AnObsoleteMicAbstractDelimiter AnObsoleteMicAnchorReferenceCloserDelimiter AnObsoleteMicAnchorReferenceOpenerDelimiter AnObsoleteMicAnnotationCloserDelimiter AnObsoleteMicAnnotationOpenerDelimiter AnObsoleteMicBoldDelimiter AnObsoleteMicFigureNameOpenerDelimiter AnObsoleteMicItalicDelimiter AnObsoleteMicLinkNameDelimiter AnObsoleteMicLinkNameCloserDelimiter AnObsoleteMicLinkNameOpenerDelimiter AnObsoleteMicMathDelimiter AnObsoleteMicMonospaceDelimiter AnObsoleteMicRawCloserDelimiter AnObsoleteMicRawOpenerDelimiter AnObsoleteMicStrikeDelimiter AnObsoleteMicElement AnObsoleteMicAbstractBlock AnObsoleteMicAbstractAnnotatedBlock AnObsoleteMicAnnotatedBlock AnObsoleteMicContinuousMarkedBlock AnObsoleteMicCommentBlock AnObsoleteMicQuoteBlock AnObsoleteMicTableBlock AnObsoleteMicListBlock AnObsoleteMicOrderedListBlock AnObsoleteMicUnorderedListBlock AnObsoleteMicListItemBlock AnObsoleteMicParagraphBlock AnObsoleteMicRootBlock AnObsoleteMicSingleLineBlock AnObsoleteMicAnchorBlock AnObsoleteMicHeaderBlock AnObsoleteMicHorizontalLineBlock AnObsoleteMicStartStopMarkupBlock AnObsoleteMicEnvironmentBlock AnObsoleteMicMetaDataBlock AnObsoleteMicSameStartStopMarkupBlock AnObsoleteMicAbstractCodeBlock AnObsoleteMicScriptBlock AnObsoleteMicPharoEvaluatorBlock AnObsoleteMicMathBlock AnObsoleteMicInlineElement AnObsoleteMicAnnotationBlock AnObsoleteMicInlineBlockWithUrl AnObsoleteMicFigureBlock AnObsoleteMicLinkBlock AnObsoleteMicTextBlock AnObsoleteMicInlineParser AnObsoleteMicrodownParser AnObsoleteMicRichTextBrush AnObsoleteMicRichTextIndentBrush AnObsoleteMicRichTextCanvas AnObsoleteMicRichTextCodeBlockStyler AnObsoleteMicTextStyler AnObsoleteMicRichTextComposer AnObsoleteMicRichTextDoIt AnObsoleteMicMorphicTextAdapter AnObsoleteMicMorphicMicrodownAdapter AnObsoleteMicScrolledTextMorph)
ReleaseTest(TestAsserter)>>assert:description:resumable:
ReleaseTest(TestAsserter)>>assert:description:
ReleaseTest>>testObsoleteClasses ...assert: obsoleteClasses isEmpty
		description: [
			String streamContents: [ :s|
				s
					nextPutAll: 'Obsolete classes remaining: ';
					print: obsoleteClasses ]]
ReleaseTest(TestCase)>>performTest


###########
# Summary #
###########

ReleaseTest
 ✗ #testUndeclared (151ms)
 ✗ #testObsoleteClasses (831ms)


  Executed 765 Tests with 1 Failures and 1 Errors in 62.56s.

[jordanmontt]

Hello @Ducasse

All the tests are now green for Microdown, yes. The problem is that there are 2 tests that will fail when we will integrate this PR, because they depend on the old asString.

What I propose is:

1. integrate this PR. This will cause the Pharo CI to have 2 failing tests because of Microdown.
2. Integrate the PR in Microdown Using new method extractInteger for parsing instead of asInteger pillar-markup/Microdown#901 This will fix the broken tests for Microdown
3. Update Pharo to use latests microdown
4. Everything, both Pharo and Microdown will be green at this point.