Repository navigation
[patch] Count every line break in HasMinimumLines, HasMaximumLines and HasExactLines - #394
Merged
Merged
Conversation
…d HasExactLines The three Has*Lines attributes counted only '\n', so a lone '\r', U+2028 or U+2029 added no line, while IsMultiLine and IsSingleLine treated '\r' and U+2028 as breaks. "a\rb" was multi-line yet had one line, so [HasMaximumLines(1)] accepted strings [IsSingleLine] rejected. The CRLF block meant to stop double counting computed the same number it started with. A shared LineBreaks helper now defines a break once: '\n', '\r', U+2028 and U+2029, with "\r\n" counting as one. All five line attributes use it, and the no-op CRLF block is gone. Fixes #299 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01FtvW1bPVVfCTF7fCWpeyRE
SonarCloud S127: CountLines skipped the '\n' of a "\r\n" pair by incrementing the for-loop variable in its body. It now leaves the counter alone and skips a '\n' whose previous character is '\r'. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01FtvW1bPVVfCTF7fCWpeyRE
|
This was referenced Oct 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #299
Problem
HasMinimumLines,HasMaximumLinesandHasExactLinescounted only\n.IsMultiLineandIsSingleLinealso treat\rand U+2028 as breaks. So"a\rb"passed[IsMultiLine]but counted as one line:[HasMaximumLines(1)]accepted strings that[IsSingleLine]rejected, and[IsMultiLine, HasMinimumLines(2)]contradicted itself. The CRLF "don't double count" block worked out to the same number it started with.Change
LineBreakshelper inSemantics.Strings/Validation/:IsLineBreak(char)returns true for\n,\r, U+2028 (line separator) and U+2029 (paragraph separator).CountLines(string)counts\r\nas one break and every other break character as one each.Has*Linesattributes useCountLines, and the no-op CRLF block is removed.IsMultiLineandIsSingleLineuseIsLineBreak, so all five attributes share one definition.Behavior change to review: the issue lists U+2029 as a break, so I included it. As a result,
IsSingleLinenow rejects U+2029 andIsMultiLinenow accepts it. Before this PR, neither treated it as a break. If you'd rather leave U+2029 out, delete theParagraphSeparatorcase fromIsLineBreakand the two U+2029DataRows.Tests (
LineCountValidatorsTests)EveryLineEndingCountsAsOneBreakchecks\r,\n,\r\n, U+2028 and U+2029: each gives exactly 2 lines inHasExactLines(2),HasMinimumLines(2)andHasMaximumLines(2), and failsHasMaximumLines(1).ConsecutiveCrlfPairsAreSeparateBreakschecks that"a\r\n\r\nb"is 3 lines and"a\n\rb\r\nc"is 4.LineCountsAgreeWithIsMultiLineAndIsSingleLinechecks that the same strings passIsMultiLineand failIsSingleLine.With the attribute changes reverted, 5 of the 14 tests in the class fail. With the changes, all pass. The full suite passes locally: 1520 passed, 8 skipped, 0 failed. The one reported error is the
Semantics.Cpp.Testnet9.0 leg, which could not start because this container has only the .NET 10 runtime. It is unrelated to this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01FtvW1bPVVfCTF7fCWpeyRE
Generated by Claude Code