Remove SafeMarkup::escape() and rely more on Twig autoescaping

+++ b/core/lib/Drupal/Component/Utility/Html.php
@@ -348,4 +348,48 @@ public static function decodeEntities($text) {
+   * Encodes all HTML entities.
...
+    return htmlspecialchars($text, ENT_QUOTES, 'UTF-8');
...
+   * Encodes all HTML entities if $text is not marked safe.

How about "Encodes special characters to HTML entities."? Maybe rename to Html::escape() or ::checkPlain()?

+++ b/core/lib/Drupal/Component/Utility/Html.php
@@ -348,4 +348,48 @@ public static function decodeEntities($text) {
+  public static function encodeEntities($text) {
...
+  public static function encodeEntitiesIfUnsafe($text) {

Why have both of these?

Cool :)

#2549791: Remove SafeMarkup::xssFilter() and provide ability to define #markup escaping strategy and what tags to filter landed.

Discussed with @catch, @alexpott, @webchick, and @effulgentsia. This issue is critical because of the unreliability, unpredictability, and confusion for using SafeMarkup::escape().

What's the relation of this issue with #2555473: Deprecate SafeMarkup::checkPlain() for Drupal 8.0.x? Do they need to be both critical? I'm asking mainly because #2545972: Remove all code usages SafeMarkup::checkPlain() and rely more on Twig autoescaping is major (sorry if this has already been answered).

+++ b/core/themes/engines/twig/twig.engine
@@ -151,29 +151,3 @@ function twig_without($element) {
-    $output .= $separator . SafeMarkup::escape($item);

OH wow, this never made sense

Just some minor tweaks: small doc fix, add typehint, fix tests by using @title instead of !title, and a unit test

Some quick expanding of the test coverage

2 nits

1. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -142,7 +142,7 @@ public function getFilters() {
   +      new \Twig_SimpleFilter('safe_join', [$this, 'safeJoin'], array('needs_environment' => true, 'is_safe' => array('html'))),
   

   please do not mix 2 array syntax

2. +++ b/core/lib/Drupal/Core/Template/TwigExtension.php
   @@ -514,4 +514,30 @@ public function renderVar($arg) {
   +  public function safeJoin(\Twig_Environment $env, $value, $glue = '') {
   +    $separator = '';
   ...
   +      $output .= $separator . $this->escapeFilter($env, $item, 'html', NULL, TRUE);
   +      $separator = $glue;
   

   I see no need in $separator

#30.2 it seems to be there because we don't want to add the separator for the first item so its an empty variable set a little earlier.

1. For such long strings better to use short-syntax as few above lines doing
2. since PHP 5.4 $this could be used in anonymous functions, and array mapping more readable

+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -530,14 +530,10 @@ public function renderVar($arg) {
-    $separator = '';
-    $output = '';
-    foreach ($value as $item) {
+    return implode($glue, array_map(function($item) use ($env) {
       // If $item is not marked safe then it will be escaped.
-      $output .= $separator . $this->escapeFilter($env, $item, 'html', NULL, TRUE);
-      $separator = $glue;
-    }
-    return $output;
+      return $this->escapeFilter($env, $item, 'html', NULL, TRUE);
+    }, $value));

Nice refactoring!

Some first idea of the change to the CR:

diff --git a/foo.txt b/foo.txt
index 516faa1..867a13a 100644
--- a/foo.txt
+++ b/foo.txt
@@ -4,6 +4,7 @@
   <li>SafeMarkup::xssFilter()</li>
   <li>SafeMarkup::placeholder() - there is no direct replacement for this. Use \Drupal\Component\Utility\SafeMarkup::format() with $args = [&#039;%placeholder&#039; =&gt; &#039;thing to be placeholdered&#039;]</li>
   <li>SafeMarkup::set() - use one of the strategies in https://www.drupal.org/node/2311123</li>
+  <li>SafeMarkup::escape() - Pretty much the same as SafeMarkup::checkPlain() applies here. Rely on twig autoescape, use #plain_text or use Html::escape() if you require explicit filtering.</li>
 </ul>
 <h3>How to filter markup</h3>
 The SafeMarkup::xssFilter() method is removed in favour of filtering using the render system. By default all #markup is filtered using the admin tag list. However the list of allowed tags can be changed using #allowed_tags. 

When this lands we will need to update #2549395: SafeMarkup methods are removed

So should we update the CR first or the other way round? This comment + tag kinda leads to a circular dependency thing :)

Patch looks good!

+++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
@@ -124,6 +127,31 @@ public function testSafeStringEscaping() {
+    SafeMarkup::setMultiple([$string => ['html' => TRUE]]);

Heh :)

+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -514,4 +514,26 @@ public function renderVar($arg) {
+   * Joins several strings together safely.
...
+   *   The pieces to join.
...
+   *   The delimiter with which to join the string. Defaults to an empty string.
...
+   * @return string
+   *   The imploded string.

First we talk about joining, then imploding, so maybe "The joined string"?

I added the suggested change for CR from @dawehner to the actual CR. The change itself looks good for me.

+++ b/core/tests/Drupal/Tests/Core/Template/TwigExtensionTest.php
@@ -124,6 +127,31 @@ public function testSafeStringEscaping() {
+
+

Extra line change can be changed while commiting

Thanks @laurii

I made a small update to the CR, maybe someone can check whether the bit about SafeMarkup::escape() still looks sane.

Committed/pushed to 8.0.x, thanks!