Skip to content

Commit 27c7c81

Browse files
jeffkljeffkl
authored
WriteLinesToFile improvements (#12707)
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](https://github.com/jeffkl/msbuild/blob/f4e7bc9c01e75873d00a36b6f6a9e3dabb771c21/src/Build/Instance/ProjectItemInstance.cs?plain=1#L927C1-L928C1) 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
1 parent 17be56c commit 27c7c81

File tree

1 file changed

+68
-69
lines changed

1 file changed

+68
-69
lines changed

‎src/Tasks/FileIO/WriteLinesToFile.cs‎

Lines changed: 68 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -60,101 +60,100 @@ public class WriteLinesToFile : TaskExtension, IIncrementalTask
6060
[Obsolete]
6161
public bool CanBeIncremental => WriteOnlyWhenDifferent;
6262

63-
/// <summary>
64-
/// Execute the task.
65-
/// </summary>
66-
/// <returns></returns>
63+
/// <inheritdoc cref="ITask.Execute" />
6764
public override bool Execute()
6865
{
6966
bool success = true;
7067

71-
if (File != null)
68+
if (File == null)
69+
{
70+
return success;
71+
}
72+
73+
string filePath = FileUtilities.NormalizePath(File.ItemSpec);
74+
75+
string contentsAsString = string.Empty;
76+
77+
if (Lines != null && Lines.Length > 0)
7278
{
73-
// do not return if Lines is null, because we may
74-
// want to delete the file in that case
75-
StringBuilder buffer = new StringBuilder();
76-
if (Lines != null)
79+
StringBuilder buffer = new StringBuilder(capacity: Lines.Length * 64);
80+
81+
foreach (ITaskItem line in Lines)
7782
{
78-
foreach (ITaskItem line in Lines)
79-
{
80-
buffer.AppendLine(line.ItemSpec);
81-
}
83+
buffer.AppendLine(line.ItemSpec);
8284
}
8385

84-
Encoding encoding = s_defaultEncoding;
85-
if (Encoding != null)
86+
contentsAsString = buffer.ToString();
87+
}
88+
89+
Encoding encoding = s_defaultEncoding;
90+
if (Encoding != null)
91+
{
92+
try
8693
{
87-
try
88-
{
89-
encoding = System.Text.Encoding.GetEncoding(Encoding);
90-
}
91-
catch (ArgumentException)
92-
{
93-
Log.LogErrorWithCodeFromResources("General.InvalidValue", "Encoding", "WriteLinesToFile");
94-
return false;
95-
}
94+
encoding = System.Text.Encoding.GetEncoding(Encoding);
95+
}
96+
catch (ArgumentException)
97+
{
98+
Log.LogErrorWithCodeFromResources("General.InvalidValue", "Encoding", "WriteLinesToFile");
99+
return false;
96100
}
101+
}
97102

98-
try
103+
try
104+
{
105+
Directory.CreateDirectory(Path.GetDirectoryName(filePath));
106+
107+
if (Overwrite)
99108
{
100-
var directoryPath = Path.GetDirectoryName(FileUtilities.NormalizePath(File.ItemSpec));
101-
if (Overwrite)
109+
// When WriteOnlyWhenDifferent is set, read the file and if they're the same return.
110+
if (WriteOnlyWhenDifferent)
102111
{
103-
Directory.CreateDirectory(directoryPath);
104-
string contentsAsString = buffer.ToString();
105-
106-
// When WriteOnlyWhenDifferent is set, read the file and if they're the same return.
107-
if (WriteOnlyWhenDifferent)
112+
MSBuildEventSource.Log.WriteLinesToFileUpToDateStart();
113+
try
108114
{
109-
MSBuildEventSource.Log.WriteLinesToFileUpToDateStart();
110-
try
115+
if (FileUtilities.FileExistsNoThrow(filePath))
111116
{
112-
if (FileUtilities.FileExistsNoThrow(File.ItemSpec))
117+
string existingContents = FileSystems.Default.ReadFileAllText(filePath);
118+
119+
if (existingContents.Equals(contentsAsString))
113120
{
114-
string existingContents = FileSystems.Default.ReadFileAllText(File.ItemSpec);
115-
if (existingContents.Length == buffer.Length)
116-
{
117-
if (existingContents.Equals(contentsAsString))
118-
{
119-
Log.LogMessageFromResources(MessageImportance.Low, "WriteLinesToFile.SkippingUnchangedFile", File.ItemSpec);
120-
MSBuildEventSource.Log.WriteLinesToFileUpToDateStop(File.ItemSpec, true);
121-
return true;
122-
}
123-
else if (FailIfNotIncremental)
124-
{
125-
Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorReadingFile", File.ItemSpec);
126-
return false;
127-
}
128-
}
121+
Log.LogMessageFromResources(MessageImportance.Low, "WriteLinesToFile.SkippingUnchangedFile", filePath);
122+
MSBuildEventSource.Log.WriteLinesToFileUpToDateStop(filePath, true);
123+
return true;
124+
}
125+
else if (FailIfNotIncremental)
126+
{
127+
Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorReadingFile", filePath);
128+
return false;
129129
}
130130
}
131-
catch (IOException)
132-
{
133-
Log.LogMessageFromResources(MessageImportance.Low, "WriteLinesToFile.ErrorReadingFile", File.ItemSpec);
134-
}
135-
MSBuildEventSource.Log.WriteLinesToFileUpToDateStop(File.ItemSpec, false);
136131
}
137-
138-
System.IO.File.WriteAllText(File.ItemSpec, contentsAsString, encoding);
139-
}
140-
else
141-
{
142-
if (WriteOnlyWhenDifferent)
132+
catch (IOException)
143133
{
144-
Log.LogMessageFromResources(MessageImportance.Normal, "WriteLinesToFile.UnusedWriteOnlyWhenDifferent", File.ItemSpec);
134+
Log.LogMessageFromResources(MessageImportance.Low, "WriteLinesToFile.ErrorReadingFile", filePath);
145135
}
146-
147-
Directory.CreateDirectory(directoryPath);
148-
System.IO.File.AppendAllText(File.ItemSpec, buffer.ToString(), encoding);
136+
MSBuildEventSource.Log.WriteLinesToFileUpToDateStop(filePath, false);
149137
}
138+
139+
System.IO.File.WriteAllText(filePath, contentsAsString, encoding);
150140
}
151-
catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e))
141+
else
152142
{
153-
string lockedFileMessage = LockCheck.GetLockedFileMessage(File.ItemSpec);
154-
Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", File.ItemSpec, e.Message, lockedFileMessage);
155-
success = false;
143+
if (WriteOnlyWhenDifferent)
144+
{
145+
Log.LogMessageFromResources(MessageImportance.Normal, "WriteLinesToFile.UnusedWriteOnlyWhenDifferent", filePath);
146+
}
147+
148+
System.IO.File.AppendAllText(filePath, contentsAsString, encoding);
156149
}
157150
}
151+
catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e))
152+
{
153+
string lockedFileMessage = LockCheck.GetLockedFileMessage(filePath);
154+
Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", filePath, e.Message, lockedFileMessage);
155+
success = false;
156+
}
158157

159158
return success;
160159
}

0 commit comments

Comments
 (0)