From 57d1aa7805dc64762ce48e62fb94302cdd33d729 Mon Sep 17 00:00:00 2001 From: Richard Webb Date: Sun, 26 Jul 2026 16:32:02 +0100 Subject: [PATCH 1/4] Add a test for 'MaxLinesInFunction' with comments at the end of the file --- .../Rules/Conventions/SourceLength.fs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs index cd960deec..b76947ac2 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs @@ -75,6 +75,23 @@ let dog x = ()""") Assert.IsFalse this.ErrorsExist + [] + member this.FunctionTooManyLinesWithMultiLineCommentAtEnd() = + this.Parse($""" +module Program + +let dog x = + (* + Foo + Bar + *) + %s{generateNewLines (FunctionLength - 4) 4} + (* + Baz + *) + ()""") + Assert.IsFalse this.ErrorsExist + [] member this.FunctionTooManyLinesWithNestsedMultiLineComment() = this.Parse($""" From 7ebc8306c6adcfb43764ff3070e9bd2efc87beb7 Mon Sep 17 00:00:00 2001 From: Richard Webb Date: Sun, 2 Aug 2026 01:15:30 +0100 Subject: [PATCH 2/4] Add some unit tests for stripMultilineComments This requires making it a top level function, rather than being nested inside checkSourceLengthRule --- src/FSharpLint.Core/AssemblyInfo.fs | 1 + .../SourceLength/SourceLengthHelper.fs | 40 +++++------ .../Rules/Conventions/SourceLength.fs | 72 +++++++++++++++++++ 3 files changed, 93 insertions(+), 20 deletions(-) diff --git a/src/FSharpLint.Core/AssemblyInfo.fs b/src/FSharpLint.Core/AssemblyInfo.fs index 464cf1ab6..9d30a1f17 100644 --- a/src/FSharpLint.Core/AssemblyInfo.fs +++ b/src/FSharpLint.Core/AssemblyInfo.fs @@ -3,5 +3,6 @@ module FSharpLint.Core.AssemblyInfo open System.Runtime.CompilerServices [] +[] () \ No newline at end of file diff --git a/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs b/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs index d2735ec43..a5c6879db 100644 --- a/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs +++ b/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs @@ -34,31 +34,31 @@ let rec private getTopLevelBalancedPairs (toProcess: List (beginIndex, index) :: getTopLevelBalancedPairs tail List.Empty | _::restOfStack -> getTopLevelBalancedPairs tail restOfStack +let internal stripMultilineComments (source: string) = + let markers = + multilineCommentMarkerRegex.Matches source + |> Seq.map (fun markerMatch -> + let index = markerMatch.Index + if source.[index] = '(' then + Begin index + else + End index) + |> Seq.sortBy (function | Begin index -> index | End index -> index) + |> Seq.toList + + getTopLevelBalancedPairs markers List.Empty + |> List.fold + (fun (currSource: string) (startIndex, endIndex) -> + let left = currSource.AsSpan(0, startIndex) + let right = currSource.AsSpan(endIndex + multilineCommentMarkerRegexCaptureGroupLength) + String.Concat(left, right)) + source + let checkSourceLengthRule (config:Config) range fileContents errorName (skipRanges: array) = let error name lineCount actual = let errorFormatString = Resources.GetString("RulesSourceLengthError") String.Format(errorFormatString, name, lineCount, actual) - let stripMultilineComments (source: string) = - let markers = - multilineCommentMarkerRegex.Matches source - |> Seq.map (fun markerMatch -> - let index = markerMatch.Index - if source.[index] = '(' then - Begin index - else - End index) - |> Seq.sortBy (function | Begin index -> index | End index -> index) - |> Seq.toList - - getTopLevelBalancedPairs markers List.Empty - |> List.fold - (fun (currSource: string) (startIndex, endIndex) -> - let left = currSource.AsSpan(0, startIndex) - let right = currSource.AsSpan(endIndex + multilineCommentMarkerRegexCaptureGroupLength) - String.Concat(left, right)) - source - match tryFindTextOfRange range fileContents with | Some(sourceCode) -> let sourceCode = diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs index b76947ac2..26bfc9e83 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs @@ -431,3 +431,75 @@ module Program let foo = "" exception SomeException of string""") Assert.IsFalse(this.ErrorExistsAt(2, 0)) + +// Tests for 'stripMultilineComments' +// ref https://github.com/fsprojects/FSharpLint/issues/869 +[] +type TestStripMultilineComments() = + + [] + member this.RemoveCommentFromStart() = + let input = """ +let dog x = + (* + Foo + Bar + *) + printf System.String.Empty + ()""" + + let expected = """ +let dog x = + + printf System.String.Empty + ()""" + + let actual = FSharpLint.Rules.Helper.SourceLength.stripMultilineComments input + Assert.AreEqual(expected, actual) + + [] + member this.RemoveCommentFromEnd() = + let input = """ +let dog x = + printf System.String.Empty + (* + Baz + *) + ()""" + + let expected = """ +let dog x = + printf System.String.Empty + + ()""" + + let actual = FSharpLint.Rules.Helper.SourceLength.stripMultilineComments input + Assert.AreEqual(expected, actual) + + [] + member this.RemoveMultipleComments() = + let input = """ +let dog x = + (* + Foo (* baz *) + let (*) = id + Bar + *) + let (*) a b = a + b + printf System.String.Empty + (* + Baz + *) + ()""" + + let expected = """ +let dog x = + + let (*) a b = a + b + printf System.String.Empty + + ()""" + + let actual = FSharpLint.Rules.Helper.SourceLength.stripMultilineComments input + Assert.AreEqual(expected, actual) + From 1f3ed39d5f87949960e233f3598d309d6634c919 Mon Sep 17 00:00:00 2001 From: Richard Webb Date: Sun, 2 Aug 2026 01:53:35 +0100 Subject: [PATCH 3/4] Rework stripMultilineComments to remove block comments in reverse order This avoids issues where removing one comment changes the offsets of following comments, including some cases where that can result in ArgumentOutOfRangeException due to string operations running off the end of the string --- .../SourceLength/SourceLengthHelper.fs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs b/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs index a5c6879db..9aa5d7656 100644 --- a/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs +++ b/src/FSharpLint.Core/Rules/Conventions/SourceLength/SourceLengthHelper.fs @@ -1,6 +1,7 @@ module FSharpLint.Rules.Helper.SourceLength open System +open System.Text open System.Text.RegularExpressions open FSharpLint.Framework open FSharpLint.Framework.Suggestion @@ -46,13 +47,20 @@ let internal stripMultilineComments (source: string) = |> Seq.sortBy (function | Begin index -> index | End index -> index) |> Seq.toList - getTopLevelBalancedPairs markers List.Empty - |> List.fold - (fun (currSource: string) (startIndex, endIndex) -> - let left = currSource.AsSpan(0, startIndex) - let right = currSource.AsSpan(endIndex + multilineCommentMarkerRegexCaptureGroupLength) - String.Concat(left, right)) - source + // Process block comment removal + // - If no comments, return input as is + // - If one comment, just remove it directly + // - If several comments, remove them all starting from the last, as removing them from the front changes the offsets of later ones + match getTopLevelBalancedPairs markers List.Empty with + | [] -> source + | [ (startIndex, endIndex) ] -> source.Remove(startIndex, (endIndex + multilineCommentMarkerRegexCaptureGroupLength) - startIndex ) + | pairs -> + + (pairs, StringBuilder(source)) + ||> List.foldBack + (fun (startIndex, endIndex)(currSource: StringBuilder) -> + currSource.Remove(startIndex, (endIndex + multilineCommentMarkerRegexCaptureGroupLength) - startIndex)) + |> _.ToString() let checkSourceLengthRule (config:Config) range fileContents errorName (skipRanges: array) = let error name lineCount actual = From f406b45d7de42b9c0d1df0ff4b016af877afea7d Mon Sep 17 00:00:00 2001 From: Richard Webb Date: Fri, 7 Aug 2026 15:54:33 +0100 Subject: [PATCH 4/4] Add an additional unit test for MaxLinesInUnion This previously triggered the exception from https://github.com/fsprojects/FSharpLint/issues/869 --- .../Rules/Conventions/SourceLength.fs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs index 26bfc9e83..d87aac2dc 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs @@ -371,7 +371,20 @@ let UnionLength = 500 [] type TestMaxLinesInUnion() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(MaxLinesInUnion.rule { Config.MaxLines = UnionLength }) - // TODO: Add tests. + + // Test a Union type with an acceptable number of lines, with inline block comments + // This cased used to trip the exception described in https://github.com/fsprojects/FSharpLint/issues/869 + // but should be fixed now. + [] + member this.UnionNotTooManyLines() = + this.Parse """ +/// Represents a single group of bindings in a class with an implicit constructor +type IncrClassBindingGroup = + | IncrClassBindingGroup of Tast.Binding list * (*isStatic:*) bool* (*recursive:*) bool + | IncrClassDo of Expr * (*isStatic:*) bool +""" + + Assert.IsFalse(this.ErrorExistsAt(4, 5)) [] let RecordLength = 500