From d4c3c773b970151ae8813bdce91bfcb984f73d3b Mon Sep 17 00:00:00 2001 From: Oryan Date: Wed, 4 Nov 2026 13:51:35 -0500 Subject: [PATCH 1/3] Fail the build on issue numbers and history in comments CommentConventionsTest scans the comments in src/main and src/test and fails on an issue number outside a `// TODO in X.0.0 (#N)`, on history narration such as "as before" or "no longer", and on "@since" or "since X.Y". It matches each comment block as one text, so a phrase split across lines is caught too. Each failure names the file and line, the rule, the matched phrase and the fix. A `lint-allow until YYYY-MM-DD by : ` marker on the line suppresses one rule for at most 90 days. An expired, reasonless or malformed marker fails. Removes the two comments that broke the rules. SchemaClassScannerInterfaceFieldTest already pins the scan order the scanner comment explained. Co-Authored-By: Claude --- .../kickstart/tools/SchemaClassScanner.kt | 3 - .../kickstart/tools/CommentConventionsTest.kt | 224 ++++++++++++++++++ .../tools/SchemaClassScannerDirectiveTest.kt | 1 - 3 files changed, 224 insertions(+), 4 deletions(-) create mode 100644 src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 5a63f30d..42f312e2 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -123,9 +123,6 @@ internal class SchemaClassScanner( handleDictionaryTypes(getAllObjectTypeMembersOfDiscoveredUnions()) { "Object type '${it.name}' is a member of a known union, but no class could be found for that type name. Please pass a class for type '${it.name}' in the parser's dictionary." } if (scanQueue()) continue - // Look for unions and interfaces used only by interface fields last, so types that can be reached some other way - // are still discovered in the same order as before. With includeUnusedTypes the unused types loop below already - // picks these up one at a time, scanning in between, so leave that path unchanged. if (!options.includeUnusedTypes && handleInterfaceFieldTypes()) continue break diff --git a/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt b/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt new file mode 100644 index 00000000..eababb4a --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt @@ -0,0 +1,224 @@ +package graphql.kickstart.tools + +import org.junit.Assert.assertEquals +import org.junit.Assert.fail +import org.junit.Test +import java.io.File +import java.time.LocalDate + +class CommentConventionsTest { + + @Test + fun `comments carry no issue numbers, history or versions`() { + val violations = listOf(File("src/main"), File("src/test")).asSequence() + .flatMap { it.walkTopDown() } + .filter { it.isFile && it.extension in setOf("kt", "java") } + .sortedBy { it.path } + .flatMap { CommentLint.check(it.path, it.readText(), LocalDate.now()) } + .toList() + + if (violations.isNotEmpty()) { + fail(violations.joinToString("\n", "${violations.size} comment(s) break the comment conventions:\n")) + } + } + + @Test + fun `lint matches whole comment blocks and honors only valid markers`() { + fun rules(source: String) = CommentLint.check("Example.kt", source, LocalDate.of(2026, 11, 4)).map { it.rule } + + assertEquals(listOf("history"), rules("/* this used to\n * be slow */")) + assertEquals(listOf("history"), rules("// this used to\n// be slow")) + assertEquals(listOf("issue-number"), rules("// see #999")) + assertEquals(listOf("version"), rules("/** @since 15.0 */")) + assertEquals(emptyList(), rules("// TODO reject in 16.0.0 (#999)")) + assertEquals(emptyList(), rules("val s = \"// see #999\"")) + assertEquals(emptyList(), rules("// unchanged lint-allow history until 2026-12-01 by oryan: domain term")) + assertEquals(listOf("lint-allow", "history"), rules("// unchanged lint-allow history until 2026-11-03 by oryan: expired")) + assertEquals(listOf("lint-allow", "history"), rules("// unchanged lint-allow history until 2026-12-01 by oryan:")) + assertEquals(listOf("lint-allow", "history"), rules("// unchanged lint-allow history until 2027-06-01 by oryan: too late")) + assertEquals(listOf("history"), rules("// unchanged lint-allow version until 2026-12-01 by oryan: other rule")) + } +} + +internal object CommentLint { + + class Violation(val path: String, val line: Int, val rule: String, val match: String, val lineText: String, val fix: String) { + override fun toString() = "$path:$line [$rule] \"$match\" in: $lineText\n fix: $fix" + } + + private class Rule(val id: String, val pattern: Regex, val fix: String) + + private const val MOVE_TO_PR = "move the reason into the PR description" + + private val historyPhrases = listOf( + """\bused to be\b""", + """\bunchanged\b""", + """\bpreviously\b""", + """\bno longer\b""", + """\bany ?more\b""", + """\bbefore (?:this|the|that) (?:change|check|fix|pr|commit)\b""", + """\b(?:before|after|until|since) #\d+""", + """\bas (?:it|they) did\b""", + """\b(?:as|like|than) before\b""", + """\bstill (?:builds?|works?|building|working)\b""", + """\bkeeps? (?:those |these |such |existing |old )?(?:schemas? )?(?:building|working)\b""", + """\bworkaround for\b""", + """\b(?:is|are|does|do|returns?|throws?) now\b""", + """\bnow (?:also |always |only )?(?:returns?|throws?|fails?|builds?|keeps?|logs?|warns?|works?|supports?)\b""", + ) + + private val rules = listOf( + Rule( + "issue-number", + Regex("""(? in X.0.0 (#N)`" + ), + Rule( + "history", + Regex(historyPhrases.joinToString("|"), RegexOption.IGNORE_CASE), + "say what the code does and why, not how it got here; $MOVE_TO_PR" + ), + Rule( + "version", + Regex("""@since\b|\bsince (?:v|version )?\d+\.\d+""", RegexOption.IGNORE_CASE), + "drop the version; history goes in the release notes" + ), + ) + + private const val MARKER_MAX_DAYS = 90L + private val marker = Regex("""lint-allow.*""") + private val validMarker = Regex("""lint-allow ([\w-]+) until (\d{4}-\d{2}-\d{2}) by [\w-]+: \S.*""") + private val markerFix = "reword the comment and drop the marker, or write `lint-allow until YYYY-MM-DD by : " + + "` with a rule from ${rules.joinToString { it.id }} and a date at most $MARKER_MAX_DAYS days ahead" + private val todoIssue = Regex("""\bTODO\b[^#]*?\bin \d+\.0\.0 (\(#\d+\))""") + + fun check(path: String, source: String, today: LocalDate): List = + CommentLexer(source).blocks().flatMap { check(path, it, today) } + + private fun check(path: String, block: List>, today: LocalDate): List { + val violations = mutableListOf() + val allowed = mutableMapOf() + val lineStarts = mutableListOf() + val text = StringBuilder() + block.forEach { (line, lineText) -> + val found = marker.find(lineText) + if (found != null) { + val rule = allowedRule(found.value, today) + if (rule == null) violations += Violation(path, line, "lint-allow", found.value, lineText, markerFix) + else allowed[line] = rule + } + if (text.isNotEmpty()) text.append(' ') + lineStarts += text.length + text.append(if (found == null) lineText else lineText.substring(0, found.range.first)) + } + val checked = todoIssue.replace(text) { + val issue = it.groupValues[1] + it.value.dropLast(issue.length) + " ".repeat(issue.length) + } + rules.forEach { rule -> + rule.pattern.findAll(checked).forEach { match -> + val (line, lineText) = block[lineStarts.indexOfLast { it <= match.range.first }] + if (allowed[line] != rule.id) violations += Violation(path, line, rule.id, match.value, lineText, rule.fix) + } + } + return violations.sortedBy { it.line } + } + + private fun allowedRule(marker: String, today: LocalDate): String? { + val match = validMarker.matchEntire(marker) ?: return null + val until = runCatching { LocalDate.parse(match.groupValues[2]) }.getOrNull() ?: return null + val rule = match.groupValues[1] + val inWindow = !until.isBefore(today) && !until.isAfter(today.plusDays(MARKER_MAX_DAYS)) + return rule.takeIf { inWindow && rules.any { it.id == rule } } + } +} + +private class CommentLexer(private val source: String) { + private var pos = 0 + private var line = 1 + private var lastLineCommentLine = -1 + private val blocks = mutableListOf>>() + + fun blocks(): List>> { + code(insideTemplate = false) + return blocks.map { block -> block.filter { it.second.isNotEmpty() } }.filter { it.isNotEmpty() } + } + + private fun peek(text: String) = source.startsWith(text, pos) + + private fun advance(count: Int = 1) { + repeat(count) { + if (pos < source.length && source[pos] == '\n') line++ + pos++ + } + } + + private fun code(insideTemplate: Boolean) { + var depth = 0 + while (pos < source.length) { + when { + peek("//") -> lineComment() + peek("/*") -> blockComment() + peek("\"\"\"") -> rawString() + peek("\"") -> string() + peek("'") -> quoted('\'') + peek("`") -> quoted('`') + insideTemplate && peek("{") -> { depth++; advance() } + insideTemplate && peek("}") -> { if (depth == 0) { advance(); return }; depth--; advance() } + else -> advance() + } + } + } + + private fun lineComment() { + val end = source.indexOf('\n', pos).let { if (it < 0) source.length else it } + val entry = line to source.substring(pos, end).trimStart('/').trim() + if (lastLineCommentLine == line - 1) blocks.last() += entry else blocks += mutableListOf(entry) + lastLineCommentLine = line + pos = end + } + + private fun blockComment() { + val startLine = line + val start = pos + advance(2) + var depth = 1 + while (pos < source.length && depth > 0) { + when { + peek("/*") -> { depth++; advance(2) } + peek("*/") -> { depth--; advance(2) } + else -> advance() + } + } + val lines = source.substring(start, pos).removePrefix("/**").removePrefix("/*").removeSuffix("*/").lines() + blocks += lines.mapIndexedTo(mutableListOf()) { index, text -> (startLine + index) to text.trim().removePrefix("*").trim() } + lastLineCommentLine = -1 + } + + private fun rawString() { + advance(3) + while (pos < source.length && !peek("\"\"\"")) { + if (peek("\${")) { advance(2); code(insideTemplate = true) } else advance() + } + advance(3) + while (peek("\"")) advance() + } + + private fun string() { + advance() + while (pos < source.length && !peek("\"") && !peek("\n")) { + when { + peek("\\") -> advance(2) + peek("\${") -> { advance(2); code(insideTemplate = true) } + else -> advance() + } + } + advance() + } + + private fun quoted(quote: Char) { + advance() + while (pos < source.length && source[pos] != quote && source[pos] != '\n') advance(if (peek("\\")) 2 else 1) + advance() + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerDirectiveTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerDirectiveTest.kt index d4be4579..34acc20b 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerDirectiveTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerDirectiveTest.kt @@ -293,7 +293,6 @@ class SchemaClassScannerDirectiveTest { assertEquals(error.message, message) } - // the enum and directive from issue #823 private val linkDirective = """ enum Purpose { SECURITY EXECUTION } From 078507b4010c03a7208214613434d899cd92af00 Mon Sep 17 00:00:00 2001 From: Oryan Date: Wed, 4 Nov 2026 14:15:00 -0500 Subject: [PATCH 2/3] Make the comment lint precise enough to trust Ordinary prose failed the build: "returns the list unchanged", "the previously resolved type", "no longer than 80 characters", "is now cached", "keeps working", "step #1", a #000000 color, "since 2.5 seconds" and a comment that mentions lint-allow. The history list keeps only phrases that narrate a change, "no longer" needs a verb of change, the version rule needs a version-shaped token, and #N skips hex colors, URL fragments and ordinals like "step #1". Text is a marker only in the full `lint-allow until` form. Every fix line names the lint-allow escape, and an invalid marker says why: expired, more than 90 days out, no reason, or an unknown rule. Java block comments no longer nest and Java strings have no templates. The test fails when it finds no sources instead of passing, and the header counts violations and lines. Co-Authored-By: Claude --- .../kickstart/tools/CommentConventionsTest.kt | 155 ++++++++++++------ 1 file changed, 108 insertions(+), 47 deletions(-) diff --git a/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt b/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt index eababb4a..a1fdb31e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt @@ -1,6 +1,7 @@ package graphql.kickstart.tools import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Assert.fail import org.junit.Test import java.io.File @@ -10,33 +11,75 @@ class CommentConventionsTest { @Test fun `comments carry no issue numbers, history or versions`() { - val violations = listOf(File("src/main"), File("src/test")).asSequence() + val sources = listOf(File("src/main"), File("src/test")).asSequence() .flatMap { it.walkTopDown() } .filter { it.isFile && it.extension in setOf("kt", "java") } .sortedBy { it.path } - .flatMap { CommentLint.check(it.path, it.readText(), LocalDate.now()) } .toList() + assertTrue("no .kt or .java files under ${File("src").absolutePath}, run the test from the project root", sources.isNotEmpty()) + val violations = sources.flatMap { CommentLint.check(it.path, it.readText(), LocalDate.now()) } if (violations.isNotEmpty()) { - fail(violations.joinToString("\n", "${violations.size} comment(s) break the comment conventions:\n")) + val lines = violations.distinctBy { it.path to it.line }.size + fail(violations.joinToString("\n", "${violations.size} comment convention violation(s) on $lines line(s):\n")) } } - @Test - fun `lint matches whole comment blocks and honors only valid markers`() { - fun rules(source: String) = CommentLint.check("Example.kt", source, LocalDate.of(2026, 11, 4)).map { it.rule } + private val today = LocalDate.of(2026, 11, 4) + + private fun check(source: String, path: String = "Example.kt") = CommentLint.check(path, source, today) + + private fun rules(source: String, path: String = "Example.kt") = check(source, path).map { it.rule } + private fun markerFix(source: String) = check(source).single { it.rule == "lint-allow" }.fix + + @Test + fun `lint flags issue numbers, history and versions in whole comment blocks`() { assertEquals(listOf("history"), rules("/* this used to\n * be slow */")) assertEquals(listOf("history"), rules("// this used to\n// be slow")) assertEquals(listOf("issue-number"), rules("// see #999")) + assertEquals(listOf("issue-number"), rules("// the enum and directive from issue #823")) + assertEquals(listOf("issue-number"), rules("// from https://github.com/o/r/issues/424")) + assertEquals(listOf("issue-number", "history"), rules("// built as the type before #552")) + assertEquals(listOf("history"), rules("// in the same order as before")) + assertEquals(listOf("history", "history"), rules("// converted, as it did before this check")) + assertEquals(listOf("history"), rules("// to keep those schemas building")) + assertEquals(listOf("history"), rules("// so those schemas keep building")) + assertEquals(listOf("history"), rules("// this check is no longer needed")) assertEquals(listOf("version"), rules("/** @since 15.0 */")) - assertEquals(emptyList(), rules("// TODO reject in 16.0.0 (#999)")) - assertEquals(emptyList(), rules("val s = \"// see #999\"")) - assertEquals(emptyList(), rules("// unchanged lint-allow history until 2026-12-01 by oryan: domain term")) - assertEquals(listOf("lint-allow", "history"), rules("// unchanged lint-allow history until 2026-11-03 by oryan: expired")) - assertEquals(listOf("lint-allow", "history"), rules("// unchanged lint-allow history until 2026-12-01 by oryan:")) - assertEquals(listOf("lint-allow", "history"), rules("// unchanged lint-allow history until 2027-06-01 by oryan: too late")) - assertEquals(listOf("history"), rules("// unchanged lint-allow version until 2026-12-01 by oryan: other rule")) + assertEquals(listOf("version"), rules("// since 1.2.0 the parser")) + assertTrue(check("// see #999 as before, since 1.2.0").all { "lint-allow" in it.fix }) + } + + @Test + fun `lint leaves ordinary prose, code and other languages' syntax alone`() { + listOf( + "// returns the list unchanged", + "// the previously resolved type wins", + "// no longer than 80 characters", + "// the result is now cached for the next call", + "// so the loop keeps working on an empty map", + "// step #1 builds the dictionary", + "// the border is #000000 and the text is #fff", + "// waits since 2.5 seconds have passed", + "// explains the lint-allow marker syntax for readers", + "// https://example.com/docs/#123", + "// TODO reject in 16.0.0 (#999)", + "val s = \"// see #999\"", + ).forEach { assertEquals(it, emptyList(), rules(it)) } + + assertEquals(listOf("issue-number"), rules("/* a /* b */\nString s = \"no longer needed\"; // see #13", "Example.java")) + assertEquals(listOf("issue-number"), rules("String s = \"\"\"\n \\\"\"\" // #1\n \"\"\"; // see #2", "Example.java")) + } + + @Test + fun `lint honors only valid markers and says why a marker is invalid`() { + assertEquals(emptyList(), rules("// as before lint-allow history until 2026-12-01 by oryan: domain term")) + assertEquals(listOf("history"), rules("// as before lint-allow version until 2026-12-01 by oryan: other rule")) + assertTrue(markerFix("// as before lint-allow history until 2026-11-03 by oryan: stale").contains("expired on 2026-11-03")) + assertTrue(markerFix("// as before lint-allow history until 2027-06-01 by oryan: too late").contains("more than 90 days")) + assertTrue(markerFix("// as before lint-allow history until 2026-12-01 by oryan:").contains("no reason")) + assertTrue(markerFix("// as before lint-allow histroy until 2026-12-01 by oryan: typo").contains("unknown rule `histroy`")) } } @@ -46,31 +89,37 @@ internal object CommentLint { override fun toString() = "$path:$line [$rule] \"$match\" in: $lineText\n fix: $fix" } - private class Rule(val id: String, val pattern: Regex, val fix: String) - + private const val MARKER_MAX_DAYS = 90L + private const val MARKER_FORM = "lint-allow until YYYY-MM-DD by : " private const val MOVE_TO_PR = "move the reason into the PR description" + private class Rule(val id: String, val pattern: Regex, advice: String) { + val fix = "$advice; for a false positive add `lint-allow $id until YYYY-MM-DD by : ` to the line, " + + "at most $MARKER_MAX_DAYS days out" + } + + private val ordinalNouns = listOf("step", "phase", "stage", "item", "case", "rule", "row", "column", "line", "attempt", "round", "part", "pass", "example", "option") + private val historyPhrases = listOf( """\bused to be\b""", - """\bunchanged\b""", - """\bpreviously\b""", - """\bno longer\b""", - """\bany ?more\b""", - """\bbefore (?:this|the|that) (?:change|check|fix|pr|commit)\b""", + """\bno longer (?:needed|necessary|required|supported|used|works?|fails?|throws?|returns?|builds?|breaks?|applies|happens|exists?|runs?)\b""", + """\banymore\b""", + """\bbefore (?:this|that) (?:change|check|fix|pr|commit)\b""", + """\bbefore the (?:fix|pr)\b""", """\b(?:before|after|until|since) #\d+""", """\bas (?:it|they) did\b""", """\b(?:as|like|than) before\b""", - """\bstill (?:builds?|works?|building|working)\b""", - """\bkeeps? (?:those |these |such |existing |old )?(?:schemas? )?(?:building|working)\b""", - """\bworkaround for\b""", - """\b(?:is|are|does|do|returns?|throws?) now\b""", - """\bnow (?:also |always |only )?(?:returns?|throws?|fails?|builds?|keeps?|logs?|warns?|works?|supports?)\b""", + """\bkeeps? (?:those|these|such|existing|old) (?:\w+ )?(?:building|working)\b""", + """\b(?:those|these|such|existing|old) schemas? (?:still|keeps?) (?:builds?|building|works?|working)\b""", ) private val rules = listOf( Rule( "issue-number", - Regex("""(? in X.0.0 (#N)`" ), Rule( @@ -80,20 +129,17 @@ internal object CommentLint { ), Rule( "version", - Regex("""@since\b|\bsince (?:v|version )?\d+\.\d+""", RegexOption.IGNORE_CASE), + Regex("""@since\b|\bsince (?:v\d+(?:\.\d+)*|version \d+(?:\.\d+)*|\d+\.\d+\.\d+)\b""", RegexOption.IGNORE_CASE), "drop the version; history goes in the release notes" ), ) - private const val MARKER_MAX_DAYS = 90L - private val marker = Regex("""lint-allow.*""") - private val validMarker = Regex("""lint-allow ([\w-]+) until (\d{4}-\d{2}-\d{2}) by [\w-]+: \S.*""") - private val markerFix = "reword the comment and drop the marker, or write `lint-allow until YYYY-MM-DD by : " + - "` with a rule from ${rules.joinToString { it.id }} and a date at most $MARKER_MAX_DAYS days ahead" + private val marker = Regex("""lint-allow [\w-]+ until\b.*""") + private val markerParts = Regex("""lint-allow ([\w-]+) until (\S+) by [\w-]+: ?(.*)""") private val todoIssue = Regex("""\bTODO\b[^#]*?\bin \d+\.0\.0 (\(#\d+\))""") fun check(path: String, source: String, today: LocalDate): List = - CommentLexer(source).blocks().flatMap { check(path, it, today) } + CommentLexer(source, kotlin = path.endsWith(".kt")).blocks().flatMap { check(path, it, today) } private fun check(path: String, block: List>, today: LocalDate): List { val violations = mutableListOf() @@ -103,9 +149,14 @@ internal object CommentLint { block.forEach { (line, lineText) -> val found = marker.find(lineText) if (found != null) { - val rule = allowedRule(found.value, today) - if (rule == null) violations += Violation(path, line, "lint-allow", found.value, lineText, markerFix) - else allowed[line] = rule + val parts = markerParts.matchEntire(found.value) + val problem = markerProblem(parts, today) + if (problem == null) allowed[line] = parts!!.groupValues[1] + else violations += Violation( + path, line, "lint-allow", found.value, lineText, + "$problem; write `$MARKER_FORM` with a rule from ${rules.joinToString { it.id }} and a date at most " + + "$MARKER_MAX_DAYS days out, or reword the comment and drop the marker" + ) } if (text.isNotEmpty()) text.append(' ') lineStarts += text.length @@ -124,16 +175,22 @@ internal object CommentLint { return violations.sortedBy { it.line } } - private fun allowedRule(marker: String, today: LocalDate): String? { - val match = validMarker.matchEntire(marker) ?: return null - val until = runCatching { LocalDate.parse(match.groupValues[2]) }.getOrNull() ?: return null - val rule = match.groupValues[1] - val inWindow = !until.isBefore(today) && !until.isAfter(today.plusDays(MARKER_MAX_DAYS)) - return rule.takeIf { inWindow && rules.any { it.id == rule } } + private fun markerProblem(parts: MatchResult?, today: LocalDate): String? { + if (parts == null) return "the marker does not have the form `$MARKER_FORM`" + val (rule, date, reason) = parts.destructured + val until = runCatching { LocalDate.parse(date) }.getOrNull() + return when { + rules.none { it.id == rule } -> "unknown rule `$rule`" + until == null -> "`$date` is not a YYYY-MM-DD date" + until.isBefore(today) -> "the marker expired on $until" + until.isAfter(today.plusDays(MARKER_MAX_DAYS)) -> "$until is more than $MARKER_MAX_DAYS days out" + reason.isBlank() -> "the marker has no reason after `by :`" + else -> null + } } } -private class CommentLexer(private val source: String) { +private class CommentLexer(private val source: String, private val kotlin: Boolean) { private var pos = 0 private var line = 1 private var lastLineCommentLine = -1 @@ -185,7 +242,7 @@ private class CommentLexer(private val source: String) { var depth = 1 while (pos < source.length && depth > 0) { when { - peek("/*") -> { depth++; advance(2) } + kotlin && peek("/*") -> { depth++; advance(2) } peek("*/") -> { depth--; advance(2) } else -> advance() } @@ -198,10 +255,14 @@ private class CommentLexer(private val source: String) { private fun rawString() { advance(3) while (pos < source.length && !peek("\"\"\"")) { - if (peek("\${")) { advance(2); code(insideTemplate = true) } else advance() + when { + !kotlin && peek("\\") -> advance(2) + kotlin && peek("\${") -> { advance(2); code(insideTemplate = true) } + else -> advance() + } } advance(3) - while (peek("\"")) advance() + while (kotlin && peek("\"")) advance() } private fun string() { @@ -209,7 +270,7 @@ private class CommentLexer(private val source: String) { while (pos < source.length && !peek("\"") && !peek("\n")) { when { peek("\\") -> advance(2) - peek("\${") -> { advance(2); code(insideTemplate = true) } + kotlin && peek("\${") -> { advance(2); code(insideTemplate = true) } else -> advance() } } From 1ffb216c9fe16c47cccf57847e4e7bd7d11253b0 Mon Sep 17 00:00:00 2001 From: Oryan Date: Wed, 4 Nov 2026 14:23:43 -0500 Subject: [PATCH 3/3] Catch keeps-building and old-behavior narration in comments "keeps building old schemas" slipped through after the precision pass narrowed the keep-building phrase to one word order. Flag keep(s) building next to schemas in either order, and old or previous behavior. Self-tests pin both, plus "used to build the type" as purpose, not history. --- .../graphql/kickstart/tools/CommentConventionsTest.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt b/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt index a1fdb31e..318997d8 100644 --- a/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/CommentConventionsTest.kt @@ -45,6 +45,10 @@ class CommentConventionsTest { assertEquals(listOf("history", "history"), rules("// converted, as it did before this check")) assertEquals(listOf("history"), rules("// to keep those schemas building")) assertEquals(listOf("history"), rules("// so those schemas keep building")) + assertEquals(listOf("history"), rules("// keeps building old schemas")) + assertEquals(listOf("history"), rules("// so schemas keep building")) + assertEquals(listOf("history"), rules("// the old behavior was to throw here")) + assertEquals(listOf("history"), rules("// restores the previous behaviour")) assertEquals(listOf("history"), rules("// this check is no longer needed")) assertEquals(listOf("version"), rules("/** @since 15.0 */")) assertEquals(listOf("version"), rules("// since 1.2.0 the parser")) @@ -56,6 +60,7 @@ class CommentConventionsTest { listOf( "// returns the list unchanged", "// the previously resolved type wins", + "// the cache used to build the type in the scanner", "// no longer than 80 characters", "// the result is now cached for the next call", "// so the loop keeps working on an empty map", @@ -111,6 +116,9 @@ internal object CommentLint { """\b(?:as|like|than) before\b""", """\bkeeps? (?:those|these|such|existing|old) (?:\w+ )?(?:building|working)\b""", """\b(?:those|these|such|existing|old) schemas? (?:still|keeps?) (?:builds?|building|works?|working)\b""", + """\bkeeps? building (?:\w+ )?schemas?\b""", + """\bschemas? keeps? building\b""", + """\b(?:old|previous) behaviou?r\b""", ) private val rules = listOf(