This would force all projects that use the method in the intended way to update their code to get rid of the deprecation message. I am not sure if they will be really happy about that.

Yes, as much as we are otherwise forcing people (e.g. Drupal) to update their code. That's why BC break are bad :)

And deprecations do never force anyone to update their code, unless they decide to do so to prepare for next major.

Sure, but Drupal is misusing the concept of safe methods. Thus IMO it's fair to have them update their code.

How do you see the method in 4.0? Will it have a completely useless parameter? How do we deal with code that triggered deprecations in 3.x? It will then still work (somehow) and the deprecation is gone?

Drupal is misusing the concept of safe methods

We have to acknowledge that we opened a big trap :-) In OSS, "the code is doc", so they just shopped for the right method and they chose right at the time they decided to use this one. Nowhere in the doc nor in the code do we mention that this method was intended to behave like RFC 7231!

How do you see the method in 4.0? Will it have a completely useless parameter?

With the code path triggering the deprecation removed. And keeping calls that have the useless param asis for cross-major-version compat yes. Then maybe deprecate any args at all in 4.1.

How do we deal with code that triggered deprecations in 3.x? It will then still work (somehow) and the deprecation is gone?

As noted in the deprecation message, code may be updated to either adding false as argument, or use isMethodCacheable depending on the use case.

If we acknowledge that the previous method was not clear enough in telling that it provided an implementation of the RFC spec, I suggest that we completely deprecate the method. We can then introduce a new one dealing with the "safe" keyword and have the isCacheableMethod() method for users who need to deal with caches.

As noted in the deprecation message, code may be updated to either adding false as argument, or use isMethodCacheable depending on the use case.

Which would be weird if you really wanted to use the method in the RFC-compatible way. Because you then for Symfony 3.x compatibility you add the false argument while you could simply omit it in 4.x again.

while you could simply omit it in 4.x again

true, we already created such migration path for 3.0, e.g. the choices_as_values form option

so they just shopped for the right method and they chose right at the time they decided to use this one

Not even that. We looked at HttpCache for Symfony best practices. Since HttpCache used Request::isMethodSafe(), we figured this was the best practice.

true, we already created such migration path for 3.0, e.g. the choices_as_values form option

That's true, but you needed to adapt your code to be compatible with Symfony 3.x. However, if you are currently only relying on the safe semantic of an HTTP method, you wouldn't have to update your code in 4.x to make it work the same. That's why I think we could then better deprecate the method itself and think about a more meaningful one (something like doesMethodHaveSideEffects()).

you wouldn't have to update your code in 4.x to make it work the same

instead of renaming, the plan could be:

• force the arg to be set and be false (but still use func_get_arg instead of real arg to preserve the signature) or throw a BadMethodCallException
• then deprecate setting this false arg in 4.1

that way, things must be explicit. WDYT?
The isMethodSafe name is consistent to me, I'd prefer keep it the same...

@xabbuh PR updated accordingly

Thank you @nicolas-grekas.

We should then at least also update the method documentation to make clear what "safe" means. Otherwise, we risk to run into the same issue again in the future.

"safe" is defined by RFC7231, and it even includes this very explicit statement:

Of the request methods defined by this specification, the GET, HEAD,
OPTIONS, and TRACE methods are defined to be safe.

https://tools.ietf.org/html/rfc7231#section-4.2.1

It is really confusing for the users when this inexplicably deviates from the spec...

The way I see it, this should have been considered a bug and fixed instead.

@teohhanhui Please have a look at the discussion in #20562.