Remove argument support from Translation Annotation

This looks wrong - we have to have a way to provide arguments when the translation happens

   public function render() {
-    return $this->t($this->string, $this->arguments, $this->options);
+    return $this->t($this->string, [], $this->options);
   }

I always think its a bad argument to argument just with "It is not used in core". From an abstract point of view TranslationWrapper delays t(), this is all, so it should support what t() gives you.

In case we remove arguments support we should explicit explain why we don't want to support arguments.

Here's a fix via pulling in some methods and tests from #2338081: Local Tasks, Actions, and Contextual links mark strings from derivatives (or alter hooks) as safe and translated

Marking critical since it's blocking the other critical issue - it doesn't make sense to proceed there without this done first (imho)

1. +++ b/core/lib/Drupal/Core/Entity/Annotation/ContentEntityType.php
   index 3a36db5..8f6236b 100644
   --- a/core/lib/Drupal/Core/StringTranslation/TranslationWrapper.php
   
   --- a/core/lib/Drupal/Core/StringTranslation/TranslationWrapper.php
   +++ b/core/lib/Drupal/Core/StringTranslation/TranslationWrapper.php
   

   As written before we should document why we not support arguments

2. +++ b/core/lib/Drupal/Core/StringTranslation/TranslationWrapper.php
   @@ -85,6 +92,16 @@ public function getOption($name) {
   +   * @return array
   

   mixed[] would be fair I guess?

3. +++ b/core/lib/Drupal/Core/StringTranslation/TranslationWrapper.php
   @@ -92,6 +109,23 @@ public function __toString() {
    
      /**
   +   * Creates a new object with updated arguments.
   +   *
   +   * @param string[] $arguments
   +   *   An array with placeholder replacements, keyed by placeholder. The values
   +   *   will be merged with and replace any existing arguments with the same
   +   *   keys.
   +   *
   +   * @return \Drupal\Core\StringTranslation\TranslationWrapper
   +   *   The new object.
   +   */
   +  public function withArguments(array $arguments) {
   +    $with = new static($this->string, $this->options);
   +    $with->arguments = $arguments + $this->arguments;
   +    return $with;
   +  }
   

   this should be not longer needed

4. +++ b/core/tests/Drupal/Tests/Core/StringTranslation/TranslationWrapperTest.php
   @@ -0,0 +1,91 @@
   +  /**
   +   * @covers ::withArguments
   +   */
   +  public function testWithArguments() {
   

   can be dropped as well

ok, let's kill it.

As long as the discussion already inckuded @effulgentsia, I think this is ready.

diff --git a/core/lib/Drupal/Core/Annotation/Translation.php b/core/lib/Drupal/Core/Annotation/Translation.php
index e1cc845..372000b 100644

index e1cc845..372000b 100644
--- a/core/lib/Drupal/Core/Annotation/Translation.php

--- a/core/lib/Drupal/Core/Annotation/Translation.php
+++ b/core/lib/Drupal/Core/Annotation/Translation.php

+++ b/core/lib/Drupal/Core/Annotation/Translation.php
@@ -73,20 +73,17 @@ class Translation extends AnnotationBase {

@@ -73,20 +73,17 @@ class Translation extends AnnotationBase {
    * @param array $values

 *
 * To provide replacement values for placeholders, use the "arguments" array:
 * @code
 *   title = @ Translation("Bundle !title", arguments = {"!title" = "Foo"}),
 * @endcode

Does someone mind removing those lines as well?

like so?

Thank you

I try to do just that change.

There we go.

change title and add the views followop.

Re the original goal of this issue, if the arguments may be dynamic and need their cache tags, so is the target language of the translations (via $options), so you would need to remove support of that too if you want predictable output based on global cache tags without the need for cache tag bubbling. At that point, the argument would be an empty or a single item array, so no need for it to be an array, so both the list of arguments and the number of them and type of them would be different between TranslationWrapper and t(), which I am not sure qualifies TranslationWrapper to be named as it is? At least someone in contrib would need to come up with an ActualTranslationWrapper that allows those options (and somehow bubbles cache info) then. Also the plan to change the signature would have required potx changes.

The newly proposed patch (totally different direction I see) may also need a sister issue in potx to adjust the parsing of the annotation.

Adding a whole bunch of tags. Given this is not required for release, and we are post API-freeze (by over two years :D), it would be good to summarize the impact / disruption like any other major API change. Also will need a change record once its settled.

Since this is just about annotation processing, is it really an API change?

Looks like there is a lack of interest to prioritize this, so removing from sprint accordingly.

With #2557113: Make t() return a TranslationWrapper object to remove reliance on a static, unpredictable safe list being critical this probably won't fly anyway :)