Skip to content

WriteLinesToFile improvements#12707

Merged
YuliiaKovalova merged 2 commits into
dotnet:mainfrom
jeffkl:improve-writelinestofile
Nov 4, 2025
Merged

WriteLinesToFile improvements#12707
YuliiaKovalova merged 2 commits into
dotnet:mainfrom
jeffkl:improve-writelinestofile

Conversation

@jeffkl

@jeffkl jeffkl commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

Fixes #

Context

I was working on something else entirely and saw some easy improvements to make in WriteLinesToFile.

Changes Made

  1. File.ItemSpec is being accessed 14 times. I think the implementation will make a new string each time.
  2. It has two code paths, overwrite vs not and both code paths were creating the directory
  3. The contents of the file to write are stored in a StringBuilder with no initial capacity. I took a wild guess and set to lines * 64
  4. If no lines are specified, a StringBuilder is no longer created and instead it uses string.Empty
  5. Since the contents are now stored in a string on all code paths, the comparison of the current vs new is simplified since string.Equals() will check the length
  6. Reduced nesting by inverting the if (File != null) statement

Testing

From what I can tell, the existing tests are good.

Notes

@jeffkl jeffkl self-assigned this Oct 29, 2025
Copilot AI review requested due to automatic review settings October 29, 2025 22:42
Copilot AI reviewed Oct 29, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the WriteLinesToFile.Execute() method to improve code readability and maintainability through better control flow structure and reducing nesting levels.

  • Inverted the null check for File to use early return
  • Extracted filePath variable from File.ItemSpec using FileUtilities.NormalizePath at the beginning of the method for consistent reuse
  • Removed redundant null checks and improved code organization by flattening nested conditions

Comment thread src/Tasks/FileIO/WriteLinesToFile.cs
Comment thread src/Tasks/FileIO/WriteLinesToFile.cs
@jeffkl jeffkl requested a review from a team October 30, 2025 16:32
@rainersigwald

rainersigwald commented Oct 30, 2025

Copy link
Copy Markdown
Member
  1. I think the implementation will make a new string each time.

Only if it's got a % in it, by my read

msbuild/src/Shared/EscapingUtilities.cs

Lines 68 to 74 in eb9c2d2

// If there are no percent signs, just return the original string immediately.
// Don't even instantiate the StringBuilder.
int indexOfPercent = escapedString.IndexOf('%');
if (indexOfPercent == -1)
{
return trim ? escapedString.Trim() : escapedString;
}

(but still no need to check that every time)

@YuliiaKovalova YuliiaKovalova merged commit 27c7c81 into dotnet:main Nov 4, 2025
16 checks passed
This was referenced Nov 5, 2025
Closed
Closed
This was referenced Nov 19, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@rainersigwald rainersigwald rainersigwald approved these changes

Assignees

@jeffkl jeffkl

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffkl @rainersigwald @YuliiaKovalova