diff --git a/skills/addressing-review-feedback/scripts/comment_growth.py b/skills/addressing-review-feedback/scripts/comment_growth.py index 903c7f5..3552350 100644 --- a/skills/addressing-review-feedback/scripts/comment_growth.py +++ b/skills/addressing-review-feedback/scripts/comment_growth.py @@ -42,6 +42,11 @@ class Family: SLASH = Family("slash", ("//",), (("/*", "*/"),), AFTER) +# A comment between JSX tags stands in braces, {/* ... */}. Only these file +# types read a line that starts with {/* as a comment: in the others it is code. +# A line that starts with "{//" or "{ //" is not a comment, because an object +# literal can open that way. +JSX = Family("jsx", ("//",), (("/*", "*/"), ("{/*", "*/"), ("{ /*", "*/")), AFTER) HASH = Family("hash", ("#",), (), AFTER) HASH_ANGLE = Family("hash-angle", ("#",), (("<#", "#>"),), AFTER) HASH_BEGIN = Family("hash-begin", ("#",), (("=begin", "=end"),), AFTER) @@ -54,7 +59,8 @@ class Family: EXTENSIONS: Dict[str, Family] = {} for _family, _extensions in ( - (SLASH, ".c .h .cc .cpp .cxx .hpp .cs .java .js .jsx .mjs .cjs .ts .tsx .go .rs .swift .kt .kts .scala .dart .php .m .mm .proto .groovy .gradle .zig .css .scss .less .v .sv"), + (SLASH, ".c .h .cc .cpp .cxx .hpp .cs .java .mjs .cjs .ts .go .rs .swift .kt .kts .scala .dart .php .m .mm .proto .groovy .gradle .zig .css .scss .less .v .sv"), + (JSX, ".js .jsx .tsx"), (HASH, ".sh .bash .zsh .fish .pl .pm .yaml .yml .toml .tf .tfvars .hcl .r .ex .exs .nix .mk .cmake .conf .cfg .properties .tcl .awk"), (HASH_ANGLE, ".ps1 .psm1"), (HASH_BEGIN, ".rb"), diff --git a/skills/addressing-review-feedback/scripts/tests/test_comment_growth.py b/skills/addressing-review-feedback/scripts/tests/test_comment_growth.py index e40de07..a597b88 100644 --- a/skills/addressing-review-feedback/scripts/tests/test_comment_growth.py +++ b/skills/addressing-review-feedback/scripts/tests/test_comment_growth.py @@ -255,6 +255,77 @@ def test_line_comments_of_these_file_types_still_count(self): self.assertEqual(self.grown(path, old, new), [(gate.GREW, 1, 2, True)]) +class JsxCommentTest(unittest.TestCase): + def scan(self, path, source): + return gate.scan(source, gate.family_for(path)) + + def grown(self, path, old, new): + family = gate.family_for(path) + return [(f.status, f.old_length, f.new_length, f.flagged) for f in gate.compare(path, old, new, family)] + + def test_comment_in_braces_on_one_line_is_a_block(self): + source = "
\n {/* about */}\n \n
\n" + for path in ("a.jsx", "a.tsx"): + with self.subTest(path=path): + self.assertEqual(self.scan(path, source), [gate.Block(2, ("{/* about */}",), "")]) + + def test_comment_in_braces_runs_to_its_closing_delimiter(self): + source = "
\n {/*\n about\n */}\n \n
\n" + self.assertEqual(self.scan("a.tsx", source), [gate.Block(2, ("{/*", "about", "*/}"), "")]) + + def test_comment_in_braces_that_grew_is_flagged(self): + old = "
\n {/* about */}\n \n
\n" + new = "
\n {/*\n about\n and more\n */}\n \n
\n" + self.assertEqual(self.grown("a.tsx", old, new), [(gate.GREW, 1, 4, True)]) + + def test_comment_in_braces_that_changed_is_listed(self): + old = "
\n {/* about */}\n \n
\n" + new = "
\n {/* other */}\n \n
\n" + self.assertEqual(self.grown("a.jsx", old, new), [(gate.CHANGED, 1, 1, False)]) + + def test_slash_comments_still_count_in_these_file_types(self): + source = "// about\nconst a = 1;\n/* more */\nconst b = 2;\n" + self.assertEqual( + self.scan("a.tsx", source), + [gate.Block(1, ("// about",), "const a = 1;"), gate.Block(3, ("/* more */",), "const b = 2;")], + ) + + def test_braces_before_a_comment_are_code_in_other_slash_file_types(self): + source = "int run(void)\n{/* empty */}\n" + for path in ("a.ts", "a.mjs", "a.cjs", "a.c"): + with self.subTest(path=path): + self.assertEqual(self.scan(path, source), []) + + def test_js_files_read_comments_in_braces(self): + source = "
\n {/* about */}\n \n
\n" + self.assertEqual(self.scan("a.js", source), [gate.Block(2, ("{/* about */}",), "")]) + + def test_comment_in_braces_after_a_space_is_a_block(self): + source = "
\n { /* about */ }\n \n
\n" + for path in ("a.js", "a.jsx", "a.tsx"): + with self.subTest(path=path): + self.assertEqual(self.scan(path, source), [gate.Block(2, ("{ /* about */ }",), "")]) + + def test_line_comment_inside_braces_is_a_block(self): + source = "
\n {\n // about\n }\n \n
\n" + for path in ("a.js", "a.jsx", "a.tsx"): + with self.subTest(path=path): + self.assertEqual(self.scan(path, source), [gate.Block(3, ("// about",), "}")]) + + def test_empty_body_that_holds_a_comment_is_code(self): + source = "function stop() {/* nothing to free */}\nstop();\n" + for path in ("a.js", "a.jsx", "a.tsx"): + with self.subTest(path=path): + self.assertEqual(self.scan(path, source), []) + + def test_object_that_opens_with_a_line_comment_is_code(self): + for opening in ("{ // home", "{// home"): + source = "const routes = [\n %s\n path: '/',\n },\n];\n" % opening + for path in ("a.js", "a.jsx", "a.tsx"): + with self.subTest(path=path, opening=opening): + self.assertEqual(self.scan(path, source), []) + + def statuses(old, new, family=gate.SLASH): return [(f.status, f.old_length, f.new_length, f.flagged) for f in gate.compare("f", old, new, family)] @@ -827,6 +898,20 @@ def test_comparison_starts_at_the_merge_base(self): sh(self.repo.path, "git", "checkout", "-q", "topic") self.assertEqual(gate.run(self.repo.path, "main").findings, ()) + def test_changed_jsx_comment_is_listed_like_a_slash_comment(self): + self.repo.write("a.tsx", "export function A() {\n return (\n
\n {/* short note */}\n \n
\n );\n}\n") + self.repo.write("b.ts", "export function B() {\n // short note\n return 1;\n}\n") + base = self.repo.commit("add a and b") + longer = "a much longer note that grew well past the first one, with more words" + for name in ("a.tsx", "b.ts"): + with open(os.path.join(self.repo.path, name), encoding="utf-8") as handle: + self.repo.write(name, handle.read().replace("short note", longer)) + report = gate.run(self.repo.path, base) + self.assertEqual( + [(f.path, f.line, f.status) for f in report.findings], + [("a.tsx", 4, gate.CHANGED), ("b.ts", 2, gate.CHANGED)], + ) + def test_untracked_file_is_scanned(self): self.repo.write("new.go", OLD) self.assertEqual(self.summary(gate.run(self.repo.path, "HEAD")), [("new.go", gate.ADDED, False)])