[0.40.0] Mistakes variable "match" for a match statement?

[hartwork]

Hi! 👋

Yapf 0.40.0 suggests this change in formatting at https://github.com/hartwork/wnpp.debian.net/actions/runs/5299147736/jobs/9591931423?pr=707#step:6:30 …

--- a/wnpp_debian_net/management/commands/importdebbugs.py
+++ b/wnpp_debian_net/management/commands/importdebbugs.py
@@ -235,8 +235,8 @@ class Command(ReportingMixin, BaseCommand):
     @staticmethod
     def _parse_wnpp_issue_subject(subject) -> tuple[str, str, str]:
         match = re.match(
-            '^(?:[Ss]ubject: )?(?P<kind>[A-Z]{1,3}): ?(?P<package>[^ ]+)(?:(?: --| -| —|:) (?P<description>.*))?$',
-            subject)
+                '^(?:[Ss]ubject: )?(?P<kind>[A-Z]{1,3}): ?(?P<package>[^ ]+)(?:(?: --| -| —|:) (?P<description>.*))?$',
+                subject)
         if match is None:
             raise _MalformedSubject(f'Malformed subject {subject!r}')

… which now indents by one level more than expected. I'm reading that as the variable "match" name being misinterpreted as a match statement. I could imagine this will be hard to fix. What do you think?

Best, Sebastian

[Spitfire1900]

@char101 are you interested in submitting a patch for this?

[char101]

$ git clone https://github.com/hartwork/wnpp.debian.net
Cloning into 'wnpp.debian.net'...
remote: Enumerating objects: 4344, done.
remote: Counting objects: 100% (707/707), done.
remote: Compressing objects: 100% (346/346), done.
remote: Total 4344 (delta 483), reused 568 (delta 361), pack-reused 3637
Receiving objects: 100% (4344/4344), 1.60 MiB | 2.26 MiB/s, done.
Resolving deltas: 100% (2773/2773), done.

$ cd wnpp.debian.net

$ ../env/bin/yapf -r -i wnpp_debian_net/management/commands/

$ git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

Either there is a bug with 3.11.4 (I still use 3.11.3) or there is a problem with pre-commit or with the workflow.

Mistaking match as a variable and a keyword is pratically impossible. Either it is treated as a regular variable (<0.40) or it is treated as a context sensitive keyword (0.40)

EDIT: Ok I can replicate it. Apparently the variable names has been renamed to match_ in the repository.

[char101]

The problem is that IsCompondStatement still look only at the keyword and not the context for match and case. The fix:

diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py
index 0c8643f..e06da09 100644
--- a/yapf/yapflib/format_decision_state.py
+++ b/yapf/yapflib/format_decision_state.py
@@ -31,6 +31,7 @@ from yapf.yapflib import logical_line
 from yapf.yapflib import object_state
 from yapf.yapflib import style
 from yapf.yapflib import subtypes
+from yapf_third_party._ylib2to3.pytree import type_repr


 class FormatDecisionState(object):
@@ -1103,8 +1104,13 @@ _COMPOUND_STMTS = frozenset({


 def _IsCompoundStatement(token):
-  if token.value == 'async':
+  value = token.value
+  if value == 'async':
     token = token.next_token
+  if value == 'match':
+    return type_repr(token.node.parent.type) == 'match_stmt'
+  if value == 'case':
+    return type_repr(token.node.parent.type) == 'case_stmt'
   return token.value in _COMPOUND_STMTS

@Spitfire1900 If you have the time please do create the pull request.

[char101]

Slightly more efficient

diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py
index 0c8643f..aada2ae 100644
--- a/yapf/yapflib/format_decision_state.py
+++ b/yapf/yapflib/format_decision_state.py
@@ -31,6 +31,7 @@ from yapf.yapflib import logical_line
 from yapf.yapflib import object_state
 from yapf.yapflib import style
 from yapf.yapflib import subtypes
+from yapf_third_party._ylib2to3.pytree import type_repr


 class FormatDecisionState(object):
@@ -1097,15 +1098,21 @@ class FormatDecisionState(object):


 _COMPOUND_STMTS = frozenset({
-    'for', 'while', 'if', 'elif', 'with', 'except', 'def', 'class', 'match',
-    'case'
+    'for', 'while', 'if', 'elif', 'with', 'except', 'def', 'class'
 })


 def _IsCompoundStatement(token):
-  if token.value == 'async':
+  value = token.value
+  if value == 'async':
     token = token.next_token
-  return token.value in _COMPOUND_STMTS
+  if token.value in _COMPOUND_STMTS:
+    return True
+  if value == 'match':
+    return type_repr(token.node.parent.type) == 'match_stmt'
+  if value == 'case':
+    return type_repr(token.node.parent.type) == 'case_stmt'
+  return False

[bwendling]

I've been trying to keep pytree-specific stuff out of the main YAPF library in yapf/yapflib. So I'm not thrilled with the use of type_repr there. There is a yapf/pytree subdirectory where you can place a check for such a thing. What do you think?

[char101]

Like this?

diff --git a/yapf/yapflib/format_decision_state.py b/yapf/yapflib/format_decision_state.py
index 0c8643f..c04a663 100644
--- a/yapf/yapflib/format_decision_state.py
+++ b/yapf/yapflib/format_decision_state.py
@@ -27,6 +27,7 @@ through the code to commit the whitespace formatting.
 """

 from yapf.pytree import split_penalty
+from yapf.pytree.pytree_utils import NodeName
 from yapf.yapflib import logical_line
 from yapf.yapflib import object_state
 from yapf.yapflib import style
@@ -1097,15 +1098,19 @@ class FormatDecisionState(object):


 _COMPOUND_STMTS = frozenset({
-    'for', 'while', 'if', 'elif', 'with', 'except', 'def', 'class', 'match',
-    'case'
+    'for', 'while', 'if', 'elif', 'with', 'except', 'def', 'class'
 })


 def _IsCompoundStatement(token):
-  if token.value == 'async':
+  value = token.value
+  if value == 'async':
     token = token.next_token
-  return token.value in _COMPOUND_STMTS
+  if token.value in _COMPOUND_STMTS:
+    return True
+  parent_name = NodeName(token.node.parent)
+  return value == 'match' and parent_name == 'match_stmt' or \
+      value == 'case' and parent_name == 'case_stmt'


 def _IsFunctionDef(token):

[bwendling]

Yes, thanks. :-)