Python: Address review feedback on template parenthesization

Remove dead `coordinates` and `cursor` params from `apply_substitutions`,
extend `_get_precedence` to handle `not` unary (precedence 3), fix test
calling renamed `apply_coordinates`, and add clarifying docstring for the
outer/inner parenthesization boundary.

Author: knutwannheden

rewrite-python/rewrite/src/rewrite/python/template/engine.py

@@ -116,17 +116,13 @@ def apply_substitutions(
         cls,
         template_tree: J,
         values: Dict[str, J],
-        cursor: 'Cursor',
-        coordinates: Optional[PythonCoordinates] = None
     ) -> Optional[J]:
         """
         Substitute placeholder identifiers with actual values.
 
         Args:
             template_tree: The parsed template AST.
             values: Dict mapping capture names to their AST values.
-            cursor: Current cursor position.
-            coordinates: Where/how to apply the template.
 
         Returns:
             The template with placeholders replaced by actual values.
@@ -135,13 +131,7 @@ def apply_substitutions(
 
         # Replace placeholders with actual values
         visitor = PlaceholderReplacementVisitor(values)
-        result = visitor.visit(template_tree, None)
-
-        # Apply coordinates (wrap in statement if needed, etc.)
-        if coordinates and result is not None:
-            result = cls.apply_coordinates(result, cursor, coordinates)
-
-        return result
+        return visitor.visit(template_tree, None)
 
     @classmethod
     def _make_cache_key(

rewrite-python/rewrite/src/rewrite/python/template/replacement.py

@@ -67,11 +67,13 @@
 
 
 def _get_precedence(expr: J) -> Optional[int]:
-    """Return the precedence of an expression, or None if not a binary op."""
+    """Return the precedence of an expression, or None if not an operator node."""
     if isinstance(expr, j.Binary):
         return _BINARY_PRECEDENCE.get(expr.operator)
     if isinstance(expr, py.Binary):
         return _BINARY_PRECEDENCE.get(expr.operator)
+    if isinstance(expr, j.Unary) and expr.operator == j.Unary.Type.Not:
+        return 3  # `not` sits between `and` (2) and comparisons (4)
     return None
 
 
@@ -111,6 +113,11 @@ def maybe_parenthesize(result: J, cursor: 'Cursor') -> J:
     """Wrap *result* in parentheses when its precedence is lower than the
     surrounding context (the cursor's parent tree node).
 
+    This handles the *outer* boundary: whether the template result as a whole
+    needs parentheses relative to where it is being inserted in the tree.
+    The *inner* operand parenthesization (within the template result itself)
+    is handled by ``PlaceholderReplacementVisitor.visit_binary`` / ``visit_unary``.
+
     This mirrors ``ParenthesizeVisitor.maybeParenthesize`` in the Java
     ``JavaTemplate`` implementation.
     """

rewrite-python/rewrite/src/rewrite/python/template/template.py

@@ -143,15 +143,14 @@ def apply(
             result = TemplateEngine.apply_substitutions(
                 template_tree,
                 values_dict,
-                cursor,
             )
         else:
             result = template_tree
 
         # Phase 2: parenthesize the result if it has lower precedence than
         # the surrounding context, mirroring JavaTemplate.doApply().
         # This must happen before coordinates are applied, because
-        # _apply_coordinates may wrap the expression in ExpressionStatement.
+        # apply_coordinates may wrap the expression in ExpressionStatement.
         if result is not None and cursor is not None:
             from .replacement import maybe_parenthesize
             result = maybe_parenthesize(result, cursor)

rewrite-python/rewrite/tests/python/template/test_engine.py

@@ -329,7 +329,7 @@ def bar(self):
         )
 
     def test_apply_coordinates_no_cu_context_does_not_crash(self):
-        """_apply_coordinates with cursor lacking CU context doesn't crash."""
+        """apply_coordinates with cursor lacking CU context doesn't crash."""
         from rewrite.visitor import Cursor
         from rewrite.python.template.coordinates import PythonCoordinates
 
@@ -340,5 +340,5 @@ def test_apply_coordinates_no_cu_context_does_not_crash(self):
         coordinates = PythonCoordinates.replace(original)
         cursor = Cursor(parent=Cursor(None, Cursor.ROOT_VALUE), value=original)
 
-        out = TemplateEngine._apply_coordinates(result, cursor, coordinates)
+        out = TemplateEngine.apply_coordinates(result, cursor, coordinates)
         assert out is not None