[ExpressionLanguage] lint should return bool

[decima]

Q	A
Branch?	7.2
Bug fix?	i don't really know
New feature?	yes and no
Deprecations?	yes?
License	MIT

I don't know how to qualify this : Documentation says lint should return boolean, but when you check the code, it's a void value, so should I update doc first, and then add it to a new branch as a new feature?

Depending on the point of view, if the documentation is the source of truth, this means it's a bug fix. But if the code is the source of truth, then, this is a new Features which introduces breaking changes 🤔

And It seems like some non-related tests in this component don't work?

[carsonbot]

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

• Always add tests
• Keep backward compatibility (see https://symfony.com/bc).
• Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
• Features and deprecations must be submitted against the 7.2 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

[xabbuh]

This indeed looks like a mistake in the documentation. The method is supposed to throw an exception in case of a syntax error. Would you like to send a PR to fix the documentation?

[decima]

This indeed looks like a mistake in the documentation. The method is supposed to throw an exception in case of a syntax error. Would you like to send a PR to fix the documentation?

Idk, I think it's weird that a method which validates an expression does't return anything. By logic, I would make it return a bool.

I bypassed my issue by doing

try {
    return $this->expressionLanguage->evaluate($condition, $args);
} catch (Exception) {
    return false;
}

I made my change on the docs repo #20351

[MrYamous]

Taking a look on it after seeing the PR doc, considering author comment from original code PR the void return is on purpose then this is pretty like a doc mistake

[derrabus]

I'm going to close this PR. The proposed change break backwards compatibility, so merging it is out of the question. I think, we agree that this is an issue of the documentation, so let's continue the discussion on symfony/symfony-docs#20351.