Skip to content

Conversation

@xperiandri
Copy link
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings July 1, 2025 13:57

This comment was marked as outdated.

@xperiandri xperiandri force-pushed the test-string-interpolation branch from 4e681e2 to cc0add2 Compare July 6, 2025 03:22
@xperiandri xperiandri changed the title Use string interpolation instead of sprintf in tests Use string interpolation instead of sprintf Jul 6, 2025
@xperiandri xperiandri requested a review from Copilot July 6, 2025 12:23
@xperiandri xperiandri force-pushed the test-string-interpolation branch from cc0add2 to 573e0ae Compare July 6, 2025 12:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces uses of sprintf and manual string concatenation with F# string interpolation across tests, core linting logic, and console output.

  • Migrate sprintf calls and + concatenations to $"..." interpolations.
  • Update functional tests, rule tests, and application code for consistent interpolation syntax.
  • Reflect changes in build script and changelog.

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/FSharpLint.FunctionalTest/TestConsoleApplication.fs Replace sprintf and concatenations with $"..." interpolations
tests/FSharpLint.Core.Tests/Rules/Typography/NoTabCharacters.fs Use String.Format and remove sprintf
tests/FSharpLint.Core.Tests/Rules/TestRuleBase.fs Replace sprintf with interpolations in test error messages
tests/FSharpLint.Core.Tests/Rules/Conventions/SourceLength.fs Migrate rule tests from sprintf to interpolated strings
tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs Replace sprintf in complexity test generators with interpolations
src/FSharpLint.Core/Rules/Hints/HintMatcher.fs Convert concatenations to interpolations in hint formatting logic
src/FSharpLint.Core/Rules/Formatting/TypedItemSpacing.fs Use interpolated string for suggested fix text
src/FSharpLint.Core/Rules/Formatting/TypePrefixing.fs Migrate type prefixing suggestions to interpolations
src/FSharpLint.Core/Rules/Formatting/TupleFormatting/TupleParentheses.fs Use $"({text})" instead of string concatenation
src/FSharpLint.Core/Rules/Conventions/SuggestUseAutoProperty.fs Replace + with interpolation in auto-property suggestion
src/FSharpLint.Core/Rules/Conventions/Naming/NamingHelper.fs Use interpolation for attribute name construction
src/FSharpLint.Core/Rules/Conventions/Binding/TupleOfWildcards.fs Use interpolation for tuple constructor formatting
src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs Migrate concatenation to interpolation in as keyword fix
src/FSharpLint.Core/Framework/Utilities.fs Use interpolated string when wrapping tuple text
src/FSharpLint.Core/Framework/HintParser.fs Convert failwith message concatenation to interpolation
src/FSharpLint.Core/Framework/AbstractSyntaxArray.fs Use interpolation in debugger display override
src/FSharpLint.Core/Application/Lint.fs Replace sprintf and + concatenations in parse error messages
src/FSharpLint.Core/Application/Configuration.fs Use interpolation for regex pattern and hint parse error
src/FSharpLint.Console/Program.fs Migrate console messaging to interpolated strings
src/FSharpLint.Console/Output.fs Replace concatenations with $"..." for output formatting
build.fsx Use interpolated strings for GitHub URLs
CHANGELOG.md Add changelog entry for string interpolation update
Comments suppressed due to low confidence (13)

tests/FSharpLint.FunctionalTest/TestConsoleApplication.fs:23

  • The interpolation still contains '%s' markers, which are invalid in an F# interpolated string. Remove '%s' and interpolate expressions directly, e.g. $"{{{Environment.NewLine} Description=\"{this.Description}\"{Environment.NewLine} ...}}".
            $"{{%s{Environment.NewLine}    Description=\"%s{this.Description}\"%s{Environment.NewLine}    Location=\"%s{this.Location}\"%s{Environment.NewLine}    Code=\"%s{this.Code}\"%s{Environment.NewLine}}}"

tests/FSharpLint.FunctionalTest/TestConsoleApplication.fs:84

  • The interpolation still contains '%s' prefixes. Remove '%s' and interpolate values directly: $"lint --lint-config {lintConfigPath} {projectFile}".
            let arguments = $"lint --lint-config %s{lintConfigPath} %s{projectFile}"

tests/FSharpLint.FunctionalTest/TestConsoleApplication.fs:97

  • The interpolation includes a '%s' marker. Interpolate the project file directly: $"lint {projectFile}".
            let arguments = $"lint %s{projectFile}"

tests/FSharpLint.FunctionalTest/TestConsoleApplication.fs:92

  • Remove '%s' from the interpolation. Use $"Output:{Environment.NewLine}{output}" instead.
            Assert.IsTrue(output.Contains("Failed while reading from config at run time"), $"Output:%s{Environment.NewLine}%s{output}")

tests/FSharpLint.FunctionalTest/TestConsoleApplication.fs:102

  • The interpolated string still has '%s'. It should be $"Could not find the file: {projectFile} on disk".
                output.Contains($"Could not find the file: %s{projectFile} on disk"),

tests/FSharpLint.FunctionalTest/TestConsoleApplication.fs:103

  • Remove the '%s' markers in the interpolation. Use $"Output:{Environment.NewLine}{output}".
                $"Output:%s{Environment.NewLine}%s{output}")

tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs:45

  • Literal '%s' inside the interpolated string is invalid. Use {NewLine} directly: $"{NewLine}elif true then ()".
    {String.replicate (len-1) $"%s{NewLine}elif true then ()"}"""

tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs:50

  • Remove '%s' in the interpolated string. Should be $"for i = 0 to 1 do (){NewLine}".
    String.replicate len $"for i = 0 to 1 do ()%s{NewLine}"

tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs:55

  • The '%s' prefix is unnecessary in the interpolation. Use $"for _ in [] do (){NewLine}".
    String.replicate len $"for _ in [] do ()%s{NewLine}"

tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs:60

  • Remove '%s' from the interpolation, e.g. $"while false do (){NewLine}".
    String.replicate len $"while false do ()%s{NewLine}"

tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs:66

  • Nested interpolation still contains '%s'. Remove it and use {NewLine} directly within the inner string.
    $"""while true && {String.replicate (len-2) $"%s{NewLine}false &&"} true do ()"""

tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs:72

  • Remove the '%s' inside the inner interpolation. Use $"{NewLine}false &&" instead.
    $"""if true && {String.replicate (len-2) $"%s{NewLine}false &&"} true then ()"""

tests/FSharpLint.Core.Tests/Rules/Conventions/CyclomaticComplexity.fs:78

  • The '%s' is still present in the nested interpolation. Drop it and interpolate {NewLine} directly.
    $"""if true || {String.replicate (len-2) $"%s{NewLine}false ||"} true then ()"""

@xperiandri xperiandri force-pushed the test-string-interpolation branch from 573e0ae to 4f6dd48 Compare July 6, 2025 13:51
@xperiandri xperiandri merged commit 46455fb into master Jul 6, 2025
7 of 8 checks passed
@xperiandri xperiandri deleted the test-string-interpolation branch July 6, 2025 14:05
let ctn =
sprintf "%s \n %s" generatorOutput.AssemblyGroup.Name (generatorOutput.AssemblyGroup.Namespaces |> Seq.map (fun n -> n.Name) |> String.concat " ")
{uri = (rootUrl + sprintf "/reference/%s/index.html" n.Label ); title = sprintf "%s - API Reference" n.Label; content = ctn }
$"%s{generatorOutput.AssemblyGroup.Name} \n %s{(generatorOutput.AssemblyGroup.Namespaces |> Seq.map (fun n -> n.Name) |> String.concat " ")}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler doesn't seem to like the nested quotes in the String.concat " " here?

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a fix up commit later

xperiandri added a commit that referenced this pull request Jul 11, 2025
## [0.25.0] - 2025-07-11

- Migrate from `Paket` to `Directory.Packages.props` #722 [@xperiandri]
- Migrate to .NET `9.0.201` and FCS `43.9.201` #722 [@xperiandri]
- Write test logs to test context output #722 [@xperiandri]
- Use string interpolation instead of `+` concatenation #724 [@xperiandri]
- Run tests in parallel #728 [@xperiandri]
- Remove `NoPartialFunctions` compiler workaround (#698) [@webwarrior-ws]
- Add `SLNX` and `SLNF` format support and migrate to SLNX solution #723 [@xperiandri]
  Remove `Ionide.ProjInfo.Sln` NuGet package dependency
- Remove `Newtonsoft.Json` NuGet dependency #725 [@xperiandri]
- Add missing rule checks for FL0079-FL0081 #713 [@BennieCopeland]
- Modify `.gitignore` to the Visual Studio standard one #735 [@xperiandri]
- Add basic Copilot instructions and GitHub MCP #726 [@xperiandri]
- Migrate `Fornax` to `0.16.0-beta002` and `FSharp.Formatting` to `20.0.1` #736 [@xperiandri, @Numpsy]
- Update the build instructions to use `dotnet fsi` instead of `fake-cli` #734 [@Numpsy]
member _.WriteInfo (info:string) = Console.Out.WriteLine info
member _.WriteWarning (warning:Suggestion.LintWarning) =
sprintf "%s(%d,%d,%d,%d):FSharpLint warning %s: %s"
fprintf Console.Out "%s(%d,%d,%d,%d):FSharpLint warning %s: %s"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xperiandri Should this be fprintfn as it used to do Out.WriteLine?

(ref #740 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants