Skip to content

Conversation

@mhsdesign
Copy link
Contributor

@mhsdesign mhsdesign commented Apr 7, 2023

Was a fun thing to do :D Thanks to your great type system

Happy Easter!

"abc" + "def"

is currently transpiled to

'abc' + 'def'

which is not valid php (should be .)

Also if we trigger a typecheck (e.g.) when using it directly as return like:

export component Foo {  
    return "abc" + "def"
}

an expression will be thrown:

Uncaught Exception: @TODO: Operand must be of type number in src/TypeSystem/Resolver/BinaryOperation/BinaryOperationTypeResolver.php:89

now it transpiles correctly:

final class Foo extends BaseClass
{
    public function render(): string
    {
        return ('abc' . 'def');
    }
}

i thought of the spec: if any of the operands is a string and the operator + is then well use string concatenation and resolve a string as return type

Comment on lines +69 to +72
if ($binaryOperationNode->operator === BinaryOperator::PLUS
&& $binaryOperationTypeResolver->resolveTypeOf($binaryOperationNode)->is(StringType::get())
) {
$operator = ' . ';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is partially weird, i initially had the logic checkIfAnyOfBinaryOperandNodesIsOfTypeString in this class, but then rembered the BinaryOperationTypeResolver should implement it.
This way we make indirectly use of it

@mhsdesign mhsdesign requested a review from grebaldi April 8, 2023 07:49
@mhsdesign
Copy link
Contributor Author

i thought of the spec: if any of the operands is a string and the operator + is then well use string concatenation and resolve a string as return type

transpiling 8 + 15 + 42 + someString to 8 . 15 . 42 . someString is actually invalid by es standard. It should equal to 65 + someString this will be no problem once: #16 is merged.

@grebaldi
Copy link
Member

We've been talking about this already, but for documentation purposes:

I think, we ought not implement string concatenation via +-operator. To explain my reasoning I'd like to cite one important statement from the "Zen of Python":

There should be one-- and preferably only one --obvious way to do it.
(see: https://peps.python.org/pep-0020/#the-zen-of-python)

The ComponentEngine has a way of doing string concatenation via template literals:

export component Foo {
  thing: string
  fortytwo: number

  return `Any${thing} is possible: ${fortytwo}`
}

I would assume that a legitimate intention behind adding string concatenation via +-operator would be to have closer resemblance (or compatibility) with ECMAScript. String concatenation in ECMAScript is a whole can of worms though. For instance, try these in your browser console:

console.log('what is the meaning of ' + 40 + 2);
console.log('what is the meaning of ' + (40 + 2)); 
console.log([] + []);
console.log(new Array(10).join(Number('String concatenation is a wonderful thing') + 'a')+' Batman!');

Therefore, I'd like to argue to leave out this ambiguity and let template literals be the only way string concatenation can be done in the ComponentEngine.

@mhsdesign
Copy link
Contributor Author

okay you win ^^ and nice one referencing the "Zen of Python" ^^

Also i think the history of js plays a role to why they have multiple ways: template strings are a newer feature and they need to stay backwards compatible with the old + way.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants