Skip to content
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

An array literal can be written with an arbitrary number of hash signs #17417

Open
koendehondt opened this issue Nov 20, 2024 · 11 comments
Open

Comments

@koendehondt
Copy link
Contributor

Bug description

An array literal is written like this:

#( 1 2 3)

But apparently Pharo allows an arbitrary number of hash signs:

##( 1 2 3)
###( 1 2 3)
####( 1 2 3)
#####( 1 2 3)
######( 1 2 3)
#######( 1 2 3)
etc

All these literals evaluate to an array with integers 1, 2, and 3.

Is that expected/desired behaviour?

@liangbing64
Copy link

also ###ABC = #ABC the answer is true

@MarcusDenker
Copy link
Member

it ignores all the # but one.

I do not like that, it accepts bad code and makes ##() un-availabe for experiments (or even usage for something)

@Ducasse
Copy link
Member

Ducasse commented Nov 23, 2024

Indeed we should fix this because it can block us in the future.

@Ducasse
Copy link
Member

Ducasse commented Nov 23, 2024

RBScanner >> scanLiteral
	"Do not allow whitespace between # and the literal."

	"Do not allow number literals after the #"

	"This scan accepts keywords as well as binaries and Identifiers."
	self step.
	characterType = #alphabetic
		ifTrue: [ ^ self scanSymbol ].
	characterType = #binary
		ifTrue: [ ^ self scanLiteralBinary ].
	currentCharacter = $'
		ifTrue: [ ^ self scanStringSymbol ].
	(currentCharacter = $( or: [ currentCharacter = $[ ])
		ifTrue: [ ^ self scanLiteralArrayToken ].
	"Accept multiple #."
	currentCharacter = $#
		ifTrue: [ ^ self scanLiteral ].
	^ self scanError: 'Literal expected'

@Ducasse
Copy link
Member

Ducasse commented Nov 23, 2024

testNextLiteralCanHaveMultipleHashtag
	| scanner token |
	scanner := self buildScannerForText: '########literal'.
	token := scanner next.
	self assert: token isLiteralToken

what a strange idea.

@Ducasse
Copy link
Member

Ducasse commented Nov 23, 2024

I did #17431

@Ducasse
Copy link
Member

Ducasse commented Nov 24, 2024

Now I cannot debug the tests :(
Because even if I change the parameterized tests to be just a group like badTokens
as soon as one is failing but this is the not the first one, running the tests from the UI rerun the first one. So I get orange then green and I have no simple idea how to debug this.

To me it looks like a strong limit of the test parameters.
I would have preferred an automatic regeneration of some test methods instead of this gigantic flury of test data where we have no tool to manage.

ASTCodeSnippetTest class >> testParameters

	^ ParametrizedTestMatrix new
		  forSelector: #snippet addOptions: ASTCodeSnippet badTokens;
		  yourself
	
	"green badScanner badMethods"
		
	"self badAnnotations.
		  self badComments.
		  self badSimpleExpressions.
		  self badExpressions.
		  self badTokens.

		  self badSemantic.
		  self badPositions.
		  self badVariableAndScopes"

@Ducasse
Copy link
Member

Ducasse commented Nov 24, 2024

Since ASTCodeSnippet badTokens size -> 52
I would have to try each of the 52.

So it feels like we should have a debug approach for the parametrized Text matrix.
I will talk to Pablo and Steven

@Ducasse
Copy link
Member

Ducasse commented Nov 24, 2024

@tesonep @StevenCostiou

@Ducasse
Copy link
Member

Ducasse commented Nov 27, 2024

@steven when I load the PR and then I checkout a branch that would remove the PR this system freezes.

@koendehondt
Copy link
Contributor Author

I forgot to add it to the initial description: GemStone does not accept literals with multiple hashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants