fix: correct duplicate-method doc mapping + comment/string-aware injection + bare-call false positive

Author: Shtirmann

osa_tool/operations/codebase/docstring_generation/insert/ts_js_augmentor.py

@@ -64,7 +64,17 @@ def _inject_methods(self, lines, methods):
         used = set()
         actions = []
 
-        for doc, meta in methods:
+        # lines inside an existing comment must never be matched as a declaration
+        # (e.g. a JSDoc body line ` * save(entity) ...` would otherwise look like a
+        # generator method `* save(`), which would corrupt the file on regeneration.
+        comment_idx = self._comment_lines(lines)
+
+        # methods may arrive out of source order (they are generated in dependency
+        # order). Map them to declarations by ascending source position so two methods
+        # sharing a name each get their OWN doc instead of a swapped one.
+        ordered = sorted(methods, key=lambda dm: dm[1].get("start_line", 0))
+
+        for doc, meta in ordered:
             name = meta["method_name"]
             patterns = [
                 # public/private/protected async static method<T>(
@@ -137,7 +147,7 @@ def _inject_methods(self, lines, methods):
             )
 
             for i, line in enumerate(lines):
-                if i in used:
+                if i in used or i in comment_idx:
                     continue
 
                 stripped = line.strip()
@@ -180,6 +190,68 @@ def _inject_methods(self, lines, methods):
 
         return lines
 
+    @staticmethod
+    def _scan_comment_state(line, in_block, quote):
+        """Advance the (in_block, quote) lexer state across one line.
+        Honours block comments `/* */`, `//` line comments, and string/template literals
+        (with backslash escapes). `quote` is threaded so a multi-line template literal
+        keeps its state and a `/*` inside it never opens a comment."""
+        i, n = 0, len(line)
+        while i < n:
+            pair = line[i : i + 2]
+            if in_block:
+                if pair == "*/":
+                    in_block = False
+                    i += 2
+                    continue
+                i += 1
+                continue
+            if quote:
+                if line[i] == "\\":
+                    i += 2
+                    continue
+                if line[i] == quote:
+                    quote = None
+                i += 1
+                continue
+            if line[i] in "\"'`":
+                quote = line[i]
+                i += 1
+                continue
+            if pair == "//":
+                break
+            if pair == "/*":
+                in_block = True
+                i += 2
+                continue
+            i += 1
+
+        # only a template literal (backtick) may legally span lines; a dangling ' or "
+        # at end of line is a mis-lexed regex/division, not a real multi-line string, so
+        # drop it instead of leaking the quote state onto the next line.
+        if quote is not None and quote != "`":
+            quote = None
+        return in_block, quote
+
+    @staticmethod
+    def _comment_lines(lines):
+        """Indices of lines whose start sits inside a block comment or a (multi-line)
+        string/template literal, or that are whole-line `//` comments, so declaration
+        patterns are never matched against comment or string text. A line that merely
+        opens a comment/string after real code is NOT marked (its code may be a
+        declaration); only lines that begin inside one are skipped."""
+        marked = set()
+        in_block = False
+        quote = None
+        for i, line in enumerate(lines):
+            if in_block or quote is not None:
+                marked.add(i)
+            elif line.strip().startswith("//"):
+                marked.add(i)
+            in_block, quote = TSJSAugmentor._scan_comment_state(line, in_block, quote)
+
+        return marked
+
     def _has_doc(self, lines, i):
         j = i - 1
         while j >= 0:

tests/unit/operations/codebase/docstring_generation/test_ts_js_augmentor.py

@@ -233,3 +233,179 @@ def test_bare_call_with_operator_not_documented():
     decl_idx = next(i for i, l in enumerate(lines) if "assert(cond)" in l)
     assert lines[decl_idx - 1].strip() == "*/"
     assert "Asserts a condition." in out
+
+
+def test_same_named_methods_mapped_by_source_order():
+    """When entries arrive out of source order (dependency order), each doc must land
+    on the method at its own start_line, not get swapped (regression for the swap bug)."""
+    aug = TSJSAugmentor()
+    src = (
+        "class A {\n\tadd(item, priority = 0) {\n\t\treturn item;\n\t}\n}\n"
+        "class B {\n\tadd(key, value) {\n\t\treturn key;\n\t}\n}\n"
+    )
+    # entries deliberately reversed (B.add first) to mimic dependency-order output
+    docs = {
+        "methods": [
+            ("Doc for B add.", {"method_name": "add", "start_line": 7}),
+            ("Doc for A add.", {"method_name": "add", "start_line": 2}),
+        ]
+    }
+    out = aug.augment("f.ts", src, docs)["f.ts"]
+    lines = out.splitlines()
+
+    a_idx = next(i for i, l in enumerate(lines) if "add(item, priority = 0)" in l)
+    b_idx = next(i for i, l in enumerate(lines) if "add(key, value)" in l)
+    a_block = "\n".join(lines[max(0, a_idx - 4) : a_idx])
+    b_block = "\n".join(lines[max(0, b_idx - 4) : b_idx])
+
+    assert "Doc for A add." in a_block
+    assert "Doc for B add." in b_block
+
+
+def test_comment_body_not_matched_as_declaration():
+    """A JSDoc body line like ` * save(entity) ...` must not be matched as the method
+    declaration (the generator `*` pattern must not hit comment lines and corrupt the
+    file). Regenerating updates the real method's doc instead."""
+    aug = TSJSAugmentor()
+    src = (
+        "class Repo {\n"
+        "\t/**\n"
+        "\t * save(entity) persists the record.\n"
+        "\t */\n"
+        "\tsave(entity) {\n"
+        "\t\treturn entity;\n"
+        "\t}\n"
+        "}\n"
+    )
+    out = aug.augment(
+        "f.ts",
+        src,
+        {"methods": [("Persists the entity.", {"method_name": "save", "start_line": 5})]},
+    )["f.ts"]
+
+    assert "Persists the entity." in out
+    # exactly one JSDoc block (the method's, replaced) -> no nested/corrupted /**
+    assert out.count("/**") == 1
+    lines = out.splitlines()
+    decl = next(i for i, l in enumerate(lines) if "save(entity) {" in l)
+    assert lines[decl - 1].strip() == "*/"
+
+
+def test_block_comment_line_not_matched_as_declaration():
+    """A plain block-comment line starting with a method name must not be matched."""
+    aug = TSJSAugmentor()
+    src = (
+        "class Repo {\n"
+        "\t/*\n"
+        "\tload(id) is deprecated, use fetch instead.\n"
+        "\t*/\n"
+        "\tload(id) {\n"
+        "\t\treturn id;\n"
+        "\t}\n"
+        "}\n"
+    )
+    out = aug.augment(
+        "f.ts",
+        src,
+        {"methods": [("Loads by id.", {"method_name": "load", "start_line": 5})]},
+    )["f.ts"]
+
+    assert "Loads by id." in out
+    # the doc must land above the real declaration, not inside the block comment
+    lines = out.splitlines()
+    decl = next(i for i, l in enumerate(lines) if "load(id) {" in l)
+    assert lines[decl - 1].strip() == "*/"
+    # the deprecated block-comment text is untouched (still present, not wrapped in /**)
+    assert "load(id) is deprecated" in out
+
+
+def test_method_with_inline_block_comment_still_documented():
+    """A method whose declaration line has an inline `/* */` comment must still be
+    documented (single-line block comments are not treated as comment lines)."""
+    aug = TSJSAugmentor()
+    src = "class C {\n\tparse(url) /* TODO */ {\n\t\treturn url;\n\t}\n}\n"
+    out = aug.augment(
+        "f.ts",
+        src,
+        {"methods": [("Parses a URL.", {"method_name": "parse", "start_line": 2})]},
+    )["f.ts"]
+
+    assert "Parses a URL." in out
+    lines = out.splitlines()
+    decl = next(i for i, l in enumerate(lines) if "parse(url)" in l)
+    assert lines[decl - 1].strip() == "*/"
+
+
+def test_string_literal_slash_star_does_not_suppress_methods():
+    """A `/*` inside a string literal must NOT open a phantom comment block that
+    suppresses documentation of later methods (literal-aware comment scan)."""
+    aug = TSJSAugmentor()
+    src = "class Repo {\n" '\tglob = "src/*";\n' "\tsave(entity) {\n\t\treturn entity;\n\t}\n" "}\n"
+    out = aug.augment(
+        "f.ts",
+        src,
+        {"methods": [("Saves the entity.", {"method_name": "save", "start_line": 3})]},
+    )["f.ts"]
+
+    assert "Saves the entity." in out
+    lines = out.splitlines()
+    decl = next(i for i, l in enumerate(lines) if "save(entity) {" in l)
+    assert lines[decl - 1].strip() == "*/"
+
+
+def test_declaration_opening_trailing_block_comment_still_documented():
+    """A declaration line that also opens a multi-line comment after the code must still
+    be documented (the code part is a real declaration)."""
+    aug = TSJSAugmentor()
+    src = "class I {\n" "\tsave(entity) { /* note:\n" "\t   multi-line */\n" "\t\treturn entity;\n" "\t}\n" "}\n"
+    out = aug.augment(
+        "f.ts",
+        src,
+        {"methods": [("Saves it.", {"method_name": "save", "start_line": 2})]},
+    )["f.ts"]
+
+    assert "Saves it." in out
+    lines = out.splitlines()
+    decl = next(i for i, l in enumerate(lines) if "save(entity)" in l and "*" != l.strip()[:1])
+    assert lines[decl - 1].strip() == "*/"
+
+
+def test_multiline_template_literal_does_not_suppress_methods():
+    """A multi-line template literal containing '/*' must not open a phantom comment
+    block (quote state is threaded across lines), so later methods stay documented."""
+    aug = TSJSAugmentor()
+    src = (
+        "class Q {\n"
+        "\tsql = `\n"
+        "\t\tSELECT /* unclosed marker\n"
+        "\t`;\n"
+        "\tsave(entity) {\n\t\treturn entity;\n\t}\n"
+        "}\n"
+    )
+    out = aug.augment(
+        "f.ts",
+        src,
+        {"methods": [("Saves it.", {"method_name": "save", "start_line": 5})]},
+    )["f.ts"]
+
+    assert "Saves it." in out
+    lines = out.splitlines()
+    decl = next(i for i, l in enumerate(lines) if "save(entity) {" in l)
+    assert lines[decl - 1].strip() == "*/"
+
+
+def test_regex_with_apostrophe_does_not_suppress_methods():
+    """A regex literal containing a lone quote char (e.g. /O'Brien/) must not leave a
+    dangling string state that suppresses documentation of later methods."""
+    aug = TSJSAugmentor()
+    src = "class M {\n" "\tre = /O'Brien/;\n" "\tsave(entity) {\n\t\treturn entity;\n\t}\n" "}\n"
+    out = aug.augment(
+        "f.ts",
+        src,
+        {"methods": [("Saves it.", {"method_name": "save", "start_line": 3})]},
+    )["f.ts"]
+
+    assert "Saves it." in out
+    lines = out.splitlines()
+    decl = next(i for i, l in enumerate(lines) if "save(entity) {" in l)
+    assert lines[decl - 1].strip() == "*/"