#805 Ignore @todo and @fixme markers in comment-not-capitalized lint#842
#805 Ignore @todo and @fixme markers in comment-not-capitalized lint#842idrisoffrinat-cpu wants to merge 3 commits intoobjectionary:masterfrom
Conversation
…alized lint The lint previously flagged any comment that didn't begin with an uppercase letter, including conventional tracker markers such as `@todo` and `@fixme`. These markers are structural, not prose, so they should be exempt. Extend the XSL predicate to skip comments matching `^@(todo|fixme)\b` (case-insensitive). The `\b` word boundary prevents accidental exemption of words like `@todolist`. Add three pack tests: - `allows-todo-marker`: standard `# @todo #NNN:NNmin ...` form - `allows-fixme-marker`: `@fixme` variant - `allows-todo-case-insensitive`: `@TODO` uppercase variant Update the motive doc with an example. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/org/eolang/lints/comments/comment-not-capitalized.xsl`:
- Line 13: The regex uses \b which XPath/XSLT fn:matches doesn't support; update
the predicate in the xsl:for-each (the fn:matches call) to avoid \b by
explicitly matching end-of-string or a non-word character — replace
"^@(todo|fixme)\b" with "^@(todo|fixme)($|[^A-Za-z0-9_])" (keep the 'i' flag) so
the exemption for `@todo/`@fixme still works without using \b.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 288c97f2-5448-4842-9937-01383ba8be58
📒 Files selected for processing (5)
src/main/resources/org/eolang/lints/comments/comment-not-capitalized.xslsrc/main/resources/org/eolang/motives/comments/comment-not-capitalized.mdsrc/test/resources/org/eolang/lints/packs/single/comment-not-capitalized/allows-fixme-marker.yamlsrc/test/resources/org/eolang/lints/packs/single/comment-not-capitalized/allows-todo-case-insensitive.yamlsrc/test/resources/org/eolang/lints/packs/single/comment-not-capitalized/allows-todo-marker.yaml
| <xsl:variable name="min" select="32"/> | ||
| <defects> | ||
| <xsl:for-each select="/object/comments/comment[not(matches(., '^[A-Z]'))]"> | ||
| <xsl:for-each select="/object/comments/comment[not(matches(., '^[A-Z]')) and not(matches(., '^@(todo|fixme)\b', 'i'))]"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for XPath regex word-boundary escapes in XSL files and for `@todolist` boundary coverage.
# Expect: no remaining '\b' in XSL regex patterns after the fix.
rg -n -C2 '\\b' -g '*.xsl'
# Expect: a negative fixture asserting `@todolist` still produces a warning.
rg -n -C3 '@todolist|todolist' src/test/resources/org/eolang/lints/packs/single/comment-not-capitalizedRepository: objectionary/lints
Length of output: 725
🌐 Web query:
In XPath/XSLT fn:matches regular expression syntax, is \b a supported word-boundary escape without Saxon’s Java-regex ;j flag?
💡 Result:
No, the \b word-boundary escape is not supported in the standard XPath/XSLT fn:matches regular expression syntax. The XPath regex dialect, based on XML Schema regex with extensions, does not include \b as a predefined escape sequence for word boundaries. Multiple sources confirm \b is invalid and causes errors in standard mode (e.g., MarkLogic throws "Invalid regular expression"; StackOverflow notes "\b isn't available in XPath"). Saxon provides \b support only as an extension when using the non-standard ;j flag to switch to Java regex engine.
Citations:
- 1: https://stackoverflow.com/questions/63826658/match-a-specific-regex-using-matches
- 2: https://general.developer.marklogic.narkive.com/sz7Ty1Hp/marklogic-dev-regular-expression-b-word-boundary
- 3: https://www.saxonica.com/html/documentation12/functions/fn/matches.html
- 4: https://saxonica.com/html/documentation/functions/fn/matches.html
- 5: http://www.regular-expressions.info/xpath.html
Replace \b with an XPath-compatible expression.
Line 13 uses \b in a regex pattern, but the word-boundary escape is not supported in standard XPath/XSLT fn:matches syntax. This will raise a regex error at runtime instead of producing lint results. Replace with an expression that explicitly checks for end-of-string or non-word characters to preserve the exemption behavior for @todolist.
Proposed fix
- <xsl:for-each select="/object/comments/comment[not(matches(., '^[A-Z]')) and not(matches(., '^@(todo|fixme)\b', 'i'))]">
+ <xsl:for-each select="/object/comments/comment[not(matches(., '^[A-Z]')) and not(matches(., '^@(todo|fixme)($|[^A-Za-z0-9_])', 'i'))]">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <xsl:for-each select="/object/comments/comment[not(matches(., '^[A-Z]')) and not(matches(., '^@(todo|fixme)\b', 'i'))]"> | |
| <xsl:for-each select="/object/comments/comment[not(matches(., '^[A-Z]')) and not(matches(., '^@(todo|fixme)($|[^A-Za-z0-9_])', 'i'))]"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/org/eolang/lints/comments/comment-not-capitalized.xsl` at
line 13, The regex uses \b which XPath/XSLT fn:matches doesn't support; update
the predicate in the xsl:for-each (the fn:matches call) to avoid \b by
explicitly matching end-of-string or a non-word character — replace
"^@(todo|fixme)\b" with "^@(todo|fixme)($|[^A-Za-z0-9_])" (keep the 'i' flag) so
the exemption for `@todo/`@fixme still works without using \b.
|
@idrisoffrinat-cpu Thanks for your contribution, we appreciate it! Before we can merge this pull request, we need to see all CI workflows pass without errors. We can't merge the pull request into the master branch unless all workflows are green. Try to fix them, also paying attention to code style issues. |
… regex Saxon's XPath 2.0 regex engine does not support \b (word boundary). Replace with [^A-Z]\$ (with the i flag) which matches the same semantics: the marker must be followed by a non-letter or be at end of string, so '@todofoo' still triggers the lint while '@todo bar' and '@todo #1234' do not. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@yegor256 the CI failures were caused by Saxon's XPath regex engine rejecting \b (word boundary) — error was 'Escape character b not allowed'. Fixed in c44a95c by replacing it with [^A-Z]|$ (with the i flag), preserving the same semantics: the marker must be followed by a non-letter or end of string. Workflows seem gated to |
3 participants
Summary
Closes #805.
The
comment-not-capitalizedlint previously flagged any comment that didn't begin with an uppercase letter — including conventional tracker markers such as@todo/@fixme. These are structural, not prose, and shouldn't be held to the same rule:Before this PR, the snippet above produced:
Fix
Extend the XPath predicate in comment-not-capitalized.xsl so comments matching
^@(todo|fixme)\b(case-insensitive) are skipped. The\bword boundary prevents@todolistor similar from being accidentally exempted.Test plan
Added three pack tests under comment-not-capitalized/:
allows-todo-marker.yaml— standard# @todo #NNN:NNmin …formallows-fixme-marker.yaml—@fixmevariantallows-todo-case-insensitive.yaml—@TODOuppercase variantThe existing
catches-not-capitalized-comment.yamlstill passes and guards against regressions on regular prose.The motive doc comment-not-capitalized.md has an example of the newly-exempt form.
Downstream impact
This unblocks this TODO in
objectionary/eoreferencing #805, which currently disables thecomment-not-capitalizedlint ineo-runtimeuntil this fix lands.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
@todo,@fixme, case-insensitive) are now exempt from capitalization requirements in linting rules.Documentation
Tests