Remove obsolete attributes from the formatter's filters and merge them

CHANGELOG.md

@@ -23,6 +23,7 @@
 
 #### :bug: Bug fix
 
+- Fix the formatter dropping a `@res.ternary` attribute written in source, e.g. `let b = @res.ternary x`, and print `@JSX` and `@res.ternary` with the same parens as other attributes, e.g. `(@JSX x) + 1`. https://github.com/rescript-lang/rescript/pull/8740
 - Respect `@warning` attributes on the module in `module type of`, e.g. `module type of @warning("-3") DeprecatedModule`, which still reported the warning. https://github.com/rescript-lang/rescript/pull/8738
 - Fix the formatter dropping or moving attributes and `await` on module expressions, such as `module M = @attr F(X)`, and printing invalid code for module constraints and functors in some positions, such as `module M: T = (X: S)`. https://github.com/rescript-lang/rescript/pull/8735
 - Make rewatch compile independent modules after an unrelated failure and recompile blocked dependents when a changed interface survives a failed implementation, including across full watcher rebuilds. https://github.com/rescript-lang/rescript/pull/8667
@@ -45,6 +46,7 @@
 
 #### :house: Internal
 
+- Remove the obsolete `res.ternary` and `JSX` attributes from the formatter's internal attribute filters and merge the filters into one. https://github.com/rescript-lang/rescript/pull/8740
 - Represent `await` on module expressions with a dedicated `Pmod_await` parsetree node instead of a `res.await` attribute, which remains only in the PPX wire representation. https://github.com/rescript-lang/rescript/pull/8738
 - Remove dead compiler code: warning 24, the `J.String_index` JS IR node, the `Pisint` and `Parraysets` Lambda primitives with their unused `%` primitive names (`%obj_is_int`, `%obj_dup`, `%obj_field`, `%array_safe_set`, `%null_to_opt`, `%nullable_to_opt`), the no-op `-pp` bsc flag and unread `Clflags` fields (`-unsafe` stays accepted as an explicit no-op), and the unused `unit` slot of `Pexp_coerce`. https://github.com/rescript-lang/rescript/pull/8731
 - Represent inline record definitions with an explicit parsetree origin while retaining the existing PPX wire representation. https://github.com/rescript-lang/rescript/pull/8686

compiler/syntax/src/res_parsetree_viewer.ml

@@ -223,8 +223,8 @@ let is_parsing_attr (attr : Parsetree.attribute) =
   match attr with
   | ( {
         Location.txt =
-          ( "res.iflet" | "res.ternary" | "res.patVariantSpread"
-          | "res.dictPattern" | "res.dictSpread" );
+          ( "res.iflet" | "res.patVariantSpread" | "res.dictPattern"
+          | "res.dictSpread" );
       },
       _ ) ->
     true
@@ -382,7 +382,7 @@ let has_attributes attrs =
   List.exists
     (fun attr ->
       match attr with
-      | {Location.txt = "res.iflet" | "res.ternary"}, _ -> false
+      | {Location.txt = "res.iflet"}, _ -> false
       (* Remove the fragile pattern warning for iflet expressions *)
       | ( {Location.txt = "warning"},
           PStr
@@ -530,25 +530,16 @@ let should_inline_rhs_binary_expr rhs =
     true
   | _ -> false
 
-let is_printable_attribute attr =
-  match attr with
-  | {Location.txt = "res.iflet" | "JSX" | "res.ternary" | "res.dictSpread"}, _
-    ->
-    false
-  | _ -> true
-
-let has_printable_attributes attrs = List.exists is_printable_attribute attrs
+let has_printable_attributes attrs =
+  List.exists (fun attr -> not (is_parsing_attr attr)) attrs
 
-(* Attributes on a module expression, the ones [Res_printer.print_attributes]
-   prints: they print before it and bind less tightly than an application or
-   a constraint *)
+(* Attributes on a module expression print before it and bind less tightly
+   than an application or a constraint *)
 let mod_expr_has_attributes (mod_expr : Parsetree.module_expr) =
-  List.exists (fun attr -> not (is_parsing_attr attr)) mod_expr.pmod_attributes
-
-let filter_printable_attributes attrs = List.filter is_printable_attribute attrs
+  has_printable_attributes mod_expr.pmod_attributes
 
 let partition_printable_attributes attrs =
-  List.partition is_printable_attribute attrs
+  List.partition (fun attr -> not (is_parsing_attr attr)) attrs
 
 let is_doc_comment_attribute ((id, payload) : Parsetree.attribute) =
   match (id, payload) with

compiler/syntax/src/res_parsetree_viewer.mli

@@ -96,7 +96,6 @@ val should_indent_binary_expr : Parsetree.expression -> bool
 val should_inline_rhs_binary_expr : Parsetree.expression -> bool
 val has_printable_attributes : Parsetree.attributes -> bool
 val mod_expr_has_attributes : Parsetree.module_expr -> bool
-val filter_printable_attributes : Parsetree.attributes -> Parsetree.attributes
 val partition_printable_attributes :
   Parsetree.attributes -> Parsetree.attributes * Parsetree.attributes
 val is_doc_comment_attribute : Parsetree.attribute -> bool

compiler/syntax/src/res_printer.ml

@@ -4290,8 +4290,7 @@ and print_binary_expression ~state ~force_pipe_breaks
                 {
                   expr with
                   pexp_attributes =
-                    Parsetree_viewer.filter_printable_attributes
-                      expr.pexp_attributes;
+                    Parsetree_viewer.filter_parsing_attrs expr.pexp_attributes;
                 }
             with
            | Braced braces_loc -> print_braces doc expr braces_loc

tests/syntax_tests/data/printer/expr/expected/markerAttributes.res.txt

@@ -0,0 +1,40 @@
+// `@JSX` and `@res.ternary` written in source are ordinary attributes: the
+// parser represents JSX and ternaries with their own nodes. They print, with
+// the same parens as any other attribute, here `@foo`.
+
+let a = (@JSX x) + 1
+let a = 1 + (@JSX x)
+let a = (@JSX x)->g
+let a = x->(@JSX g)
+let a = (@JSX x)->g(1)
+let a = (@JSX f(x)) + 1
+let a = @JSX (a + b) * c
+let a = (@JSX x) == y
+let a = (@JSX x) && y
+let a = !(@JSX x)
+let a = (@JSX x).field
+let a = @JSX x ? a : b
+
+let b = @res.ternary x
+let b = (@res.ternary x) + 1
+let b = 1 + (@res.ternary x)
+let b = (@res.ternary x)->g
+let b = x->(@res.ternary g)
+let b = (@res.ternary f(x)) + 1
+let b = @res.ternary (a + b) * c
+let b = (@res.ternary (c ? a : b)) + 1
+let b = !(@res.ternary x)
+let b = -(@res.ternary x)
+let b = (@res.ternary x).field
+let b = (@res.ternary x)[0]
+let b = @res.ternary x ? a : b
+let b = c ? @res.ternary x : b
+let b = f(@res.ternary x)
+let b = [@res.ternary x, y]
+let b = {a: @res.ternary x}
+
+let c = (@foo x) + 1
+let c = (@foo x)->g
+let c = x->(@foo g)
+let c = @foo (a + b) * c
+let c = !(@foo x)

tests/syntax_tests/data/printer/expr/markerAttributes.res

@@ -0,0 +1,40 @@
+// `@JSX` and `@res.ternary` written in source are ordinary attributes: the
+// parser represents JSX and ternaries with their own nodes. They print, with
+// the same parens as any other attribute, here `@foo`.
+
+let a = (@JSX x) + 1
+let a = 1 + (@JSX x)
+let a = (@JSX x)->g
+let a = x->(@JSX g)
+let a = (@JSX x)->g(1)
+let a = (@JSX f(x)) + 1
+let a = (@JSX (a + b)) * c
+let a = (@JSX x) == y
+let a = (@JSX x) && y
+let a = !(@JSX x)
+let a = (@JSX x).field
+let a = (@JSX x) ? a : b
+
+let b = @res.ternary x
+let b = (@res.ternary x) + 1
+let b = 1 + (@res.ternary x)
+let b = (@res.ternary x)->g
+let b = x->(@res.ternary g)
+let b = (@res.ternary f(x)) + 1
+let b = (@res.ternary (a + b)) * c
+let b = (@res.ternary (c ? a : b)) + 1
+let b = !(@res.ternary x)
+let b = -(@res.ternary x)
+let b = (@res.ternary x).field
+let b = (@res.ternary x)[0]
+let b = (@res.ternary x) ? a : b
+let b = c ? (@res.ternary x) : b
+let b = f(@res.ternary x)
+let b = [@res.ternary x, y]
+let b = {a: @res.ternary x}
+
+let c = (@foo x) + 1
+let c = (@foo x)->g
+let c = x->(@foo g)
+let c = (@foo (a + b)) * c
+let c = !(@foo x)