GH-94036: Fix more attribute location quirks (GH-95028)

3 files changed, 85 insertions, 20 deletions

diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py
index 19e47cb..11940be 100644
--- a/Lib/test/test_compile.py
+++ b/Lib/test/test_compile.py
@@ -1240,6 +1240,66 @@ f(
                     end_column=7,
                 )
 
+    def test_attribute_augassign(self):
+        source = "(\n lhs  \n   .    \n     rhs      \n       ) += 42"
+        code = compile(source, "<test>", "exec")
+        self.assertOpcodeSourcePositionIs(
+            code, "LOAD_ATTR", line=4, end_line=4, column=5, end_column=8
+        )
+        self.assertOpcodeSourcePositionIs(
+            code, "STORE_ATTR", line=4, end_line=4, column=5, end_column=8
+        )
+
+    def test_attribute_del(self):
+        source = "del (\n lhs  \n   .    \n     rhs      \n       )"
+        code = compile(source, "<test>", "exec")
+        self.assertOpcodeSourcePositionIs(
+            code, "DELETE_ATTR", line=4, end_line=4, column=5, end_column=8
+        )
+
+    def test_attribute_load(self):
+        source = "(\n lhs  \n   .    \n     rhs      \n       )"
+        code = compile(source, "<test>", "exec")
+        self.assertOpcodeSourcePositionIs(
+            code, "LOAD_ATTR", line=4, end_line=4, column=5, end_column=8
+        )
+
+    def test_attribute_store(self):
+        source = "(\n lhs  \n   .    \n     rhs      \n       ) = 42"
+        code = compile(source, "<test>", "exec")
+        self.assertOpcodeSourcePositionIs(
+            code, "STORE_ATTR", line=4, end_line=4, column=5, end_column=8
+        )
+
+    def test_method_call(self):
+        source = "(\n lhs  \n   .    \n     rhs      \n       )()"
+        code = compile(source, "<test>", "exec")
+        self.assertOpcodeSourcePositionIs(
+            code, "LOAD_ATTR", line=4, end_line=4, column=5, end_column=8
+        )
+        self.assertOpcodeSourcePositionIs(
+            code, "CALL", line=4, end_line=5, column=5, end_column=10
+        )
+
+    def test_weird_attribute_position_regressions(self):
+        def f():
+            (bar.
+        baz)
+            (bar.
+        baz(
+        ))
+            files().setdefault(
+                0
+            ).setdefault(
+                0
+            )
+        for line, end_line, column, end_column in f.__code__.co_positions():
+            self.assertIsNotNone(line)
+            self.assertIsNotNone(end_line)
+            self.assertIsNotNone(column)
+            self.assertIsNotNone(end_column)
+            self.assertLessEqual((line, column), (end_line, end_column))
+
 
 class TestExpressionStackSize(unittest.TestCase):
     # These tests check that the computed stack size for a code object
diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-07-19-16-30-59.gh-issue-94036._6Utkm.rst b/Misc/NEWS.d/next/Core and Builtins/2022-07-19-16-30-59.gh-issue-94036._6Utkm.rst
new file mode 100644
index 0000000..b0f0367
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2022-07-19-16-30-59.gh-issue-94036._6Utkm.rst
@@ -0,0 +1,2 @@
+Fix incorrect source location info for some multi-line attribute accesses
+and method calls.
diff --git a/Python/compile.c b/Python/compile.c
index 9db61fb..94ce866 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -4729,19 +4729,29 @@ is_import_originated(struct compiler *c, expr_ty e)
     return flags & DEF_IMPORT;
 }
 
+// If an attribute access spans multiple lines, update the current start
+// location to point to the attribute name.
 static void
-update_location_to_match_attr(struct compiler *c, expr_ty meth)
+update_start_location_to_match_attr(struct compiler *c, expr_ty attr)
 {
-    if (meth->lineno != meth->end_lineno) {
-        // Make start location match attribute
-        c->u->u_loc.lineno = c->u->u_loc.end_lineno = meth->end_lineno;
-        int len = (int)PyUnicode_GET_LENGTH(meth->v.Attribute.attr);
-        if (len <= meth->end_col_offset) {
-            c->u->u_loc.col_offset = meth->end_col_offset - len;
+    assert(attr->kind == Attribute_kind);
+    struct location *loc = &c->u->u_loc;
+    if (loc->lineno != attr->end_lineno) {
+        loc->lineno = attr->end_lineno;
+        int len = (int)PyUnicode_GET_LENGTH(attr->v.Attribute.attr);
+        if (len <= attr->end_col_offset) {
+            loc->col_offset = attr->end_col_offset - len;
         }
         else {
             // GH-94694: Somebody's compiling weird ASTs. Just drop the columns:
-            c->u->u_loc.col_offset = c->u->u_loc.end_col_offset = -1;
+            loc->col_offset = -1;
+            loc->end_col_offset = -1;
+        }
+        // Make sure the end position still follows the start position, even for
+        // weird ASTs:
+        loc->end_lineno = Py_MAX(loc->lineno, loc->end_lineno);
+        if (loc->lineno == loc->end_lineno) {
+            loc->end_col_offset = Py_MAX(loc->col_offset, loc->end_col_offset);
         }
     }
 }
@@ -4788,7 +4798,7 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e)
     /* Alright, we can optimize the code. */
     VISIT(c, expr, meth->v.Attribute.value);
     SET_LOC(c, meth);
-    update_location_to_match_attr(c, meth);
+    update_start_location_to_match_attr(c, meth);
     ADDOP_NAME(c, LOAD_METHOD, meth->v.Attribute.attr, names);
     VISIT_SEQ(c, expr, e->v.Call.args);
 
@@ -4799,7 +4809,7 @@ maybe_optimize_method_call(struct compiler *c, expr_ty e)
         };
     }
     SET_LOC(c, e);
-    update_location_to_match_attr(c, meth);
+    update_start_location_to_match_attr(c, meth);
     ADDOP_I(c, CALL, argsl + kwdsl);
     return 1;
 }
@@ -5811,23 +5821,18 @@ compiler_visit_expr1(struct compiler *c, expr_ty e)
     /* The following exprs can be assignment targets. */
     case Attribute_kind:
         VISIT(c, expr, e->v.Attribute.value);
+        update_start_location_to_match_attr(c, e);
         switch (e->v.Attribute.ctx) {
         case Load:
         {
-            int old_lineno = c->u->u_loc.lineno;
-            c->u->u_loc.lineno = e->end_lineno;
             ADDOP_NAME(c, LOAD_ATTR, e->v.Attribute.attr, names);
-            c->u->u_loc.lineno = old_lineno;
             break;
         }
         case Store:
             if (forbidden_name(c, e->v.Attribute.attr, e->v.Attribute.ctx)) {
                 return 0;
             }
-            int old_lineno = c->u->u_loc.lineno;
-            c->u->u_loc.lineno = e->end_lineno;
             ADDOP_NAME(c, STORE_ATTR, e->v.Attribute.attr, names);
-            c->u->u_loc.lineno = old_lineno;
             break;
         case Del:
             ADDOP_NAME(c, DELETE_ATTR, e->v.Attribute.attr, names);
@@ -5898,10 +5903,8 @@ compiler_augassign(struct compiler *c, stmt_ty s)
     case Attribute_kind:
         VISIT(c, expr, e->v.Attribute.value);
         ADDOP_I(c, COPY, 1);
-        int old_lineno = c->u->u_loc.lineno;
-        c->u->u_loc.lineno = e->end_lineno;
+        update_start_location_to_match_attr(c, e);
         ADDOP_NAME(c, LOAD_ATTR, e->v.Attribute.attr, names);
-        c->u->u_loc.lineno = old_lineno;
         break;
     case Subscript_kind:
         VISIT(c, expr, e->v.Subscript.value);
@@ -5941,7 +5944,7 @@ compiler_augassign(struct compiler *c, stmt_ty s)
 
     switch (e->kind) {
     case Attribute_kind:
-        c->u->u_loc.lineno = e->end_lineno;
+        update_start_location_to_match_attr(c, e);
         ADDOP_I(c, SWAP, 2);
         ADDOP_NAME(c, STORE_ATTR, e->v.Attribute.attr, names);
         break;