Skip to content
This repository was archived by the owner on Jul 12, 2022. It is now read-only.

Commit e221f31

Browse files
AlexGhiondeaAlexGhiondea
committed
Fix the multi-line comment bugs
There were multiple issues with the rule to remove the illegal headers: - We were using the original SyntaxTriviaList and we were making changes on top of a temporary copy. However, we *always* used the original object when iterating over the items. This meant that we were trying to find nodes based on the original list in the mutated list which lead to a crash. - When an illegal header contained an illegal header we would remove the entire multiline comment instead of just the line that contained the offending header. - We were not correctly removing tags if they appeared multiple times - Some of the comments contain the file name. Since this is changes, I created a dynamic tag that is replaced with the document name before we process it. - Updated the IllegalHeaders.md file to contain some of the common tags we saw when cleaning up mscorlib - Made the IllegalHeaders.md be dropped next to the test project so that tests could load it. - Add tests for this rule.
1 parent 2b3bd5a commit e221f31

File tree

4 files changed

+394
-13
lines changed

4 files changed

+394
-13
lines changed

‎src/CodeFormatter/IllegalHeaders.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
## Contains header comments that need to be removed from the start of file.
2+
## The <<<filename>>> entry is a dynamic header that will be replaced with the actual file name of the file.
23

34
Copyright (c) Microsoft Corporation.
45
<owner>
56
</owner>
67
==--==
78
==++==
89
<OWNER>
9-
</OWNER>
10+
</OWNER>
11+
Date:
12+
Class:
13+
File:
14+
<<<filename>>>

‎src/Microsoft.DotNet.CodeFormatting.Tests/Microsoft.DotNet.CodeFormatting.Tests.csproj

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
<AssemblyName>Microsoft.DotNet.CodeFormatting.Tests</AssemblyName>
1313
<TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
1414
<FileAlignment>512</FileAlignment>
15+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\</SolutionDir>
16+
<RestorePackages>true</RestorePackages>
1517
</PropertyGroup>
1618
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
1719
<DebugSymbols>true</DebugSymbols>
@@ -104,6 +106,7 @@
104106
<ItemGroup>
105107
<Compile Include="CodeFormattingTestBase.cs" />
106108
<Compile Include="Rules\HasNewLineBeforeFirstNamespaceFormattingRuleTests.cs" />
109+
<Compile Include="Rules\HasNoIllegalHeadersFormattingRuleTests.cs" />
107110
<Compile Include="Rules\HasNewLineBeforeFirstUsingFormattingRuleTests.cs" />
108111
<Compile Include="Rules\HasNoNewLineAfterOpenBraceFormattingRuleTests.cs" />
109112
<Compile Include="Rules\HasNoNewLineBeforeEndBraceFormattingRuleTests.cs" />
@@ -115,6 +118,10 @@
115118
<Folder Include="Properties\" />
116119
</ItemGroup>
117120
<ItemGroup>
121+
<None Include="..\CodeFormatter\IllegalHeaders.md">
122+
<Link>IllegalHeaders.md</Link>
123+
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
124+
</None>
118125
<None Include="packages.config" />
119126
</ItemGroup>
120127
<ItemGroup>
@@ -139,6 +146,7 @@
139146
</PropertyGroup>
140147
<Error Condition="!Exists('..\packages\xunit.runner.visualstudio.0.99.9-build1021\build\net20\xunit.runner.visualstudio.props')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\xunit.runner.visualstudio.0.99.9-build1021\build\net20\xunit.runner.visualstudio.props'))" />
141148
</Target>
149+
<Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
142150
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
143151
Other similar extension points exist, see Microsoft.Common.targets.
144152
<Target Name="BeforeBuild">

‎src/Microsoft.DotNet.CodeFormatting.Tests/Rules/HasNoIllegalHeadersFormattingRuleTests.cs

Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System.Collections.Generic;
5+
using Xunit;
6+
7+
namespace Microsoft.DotNet.CodeFormatting.Tests
8+
{
9+
public class HasNoIllegalHeadersFormattingRuleTests : CodeFormattingTestBase
10+
{
11+
[Fact]
12+
public void TestHasNoIllegalHeadersFormattingRule01()
13+
{
14+
var text = @"
15+
// <OWNER>test</OWNER>
16+
using System;
17+
";
18+
var expected = @"
19+
using System;
20+
";
21+
Verify(text, expected);
22+
}
23+
24+
[Fact]
25+
public void TestHasNoIllegalHeadersFormattingRule02()
26+
{
27+
var text = @"
28+
// <OWNER>test</OWNER>
29+
// <Owner>foobar</owner>
30+
using System;
31+
";
32+
var expected = @"
33+
using System;
34+
";
35+
Verify(text, expected);
36+
}
37+
38+
[Fact]
39+
public void TestHasNoIllegalHeadersFormattingRule03()
40+
{
41+
var text = @"
42+
/* <OWNER>test</OWNER>
43+
* <Owner>foobar</owner>
44+
*/
45+
using System;
46+
";
47+
var expected = @"
48+
using System;
49+
";
50+
Verify(text, expected);
51+
}
52+
53+
[Fact]
54+
public void TestHasNoIllegalHeadersFormattingRule04()
55+
{
56+
var text = @"
57+
/*
58+
* <OWNER>test</OWNER>
59+
* <Owner>foobar</owner>
60+
*/
61+
// Foobar
62+
using System;
63+
";
64+
var expected = @"
65+
// Foobar
66+
using System;
67+
";
68+
Verify(text, expected);
69+
}
70+
71+
[Fact]
72+
public void TestHasNoIllegalHeadersFormattingRule05()
73+
{
74+
var text = @"
75+
/* This is important stuff
76+
* <OWNER>test</OWNER>
77+
* <Owner>foobar</owner>
78+
*/
79+
// Foobar
80+
using System;
81+
";
82+
var expected = @"
83+
/* This is important stuff
84+
*/
85+
// Foobar
86+
using System;
87+
";
88+
Verify(text, expected);
89+
}
90+
91+
[Fact]
92+
public void TestHasNoIllegalHeadersFormattingRule06()
93+
{
94+
var text = @"
95+
/*
96+
* <OWNER>test</OWNER>
97+
* <Owner>foobar</owner>
98+
This is important stuff */
99+
// Foobar
100+
using System;
101+
";
102+
var expected = @"
103+
/*
104+
This is important stuff */
105+
// Foobar
106+
using System;
107+
";
108+
Verify(text, expected);
109+
}
110+
111+
[Fact]
112+
public void TestHasNoIllegalHeadersFormattingRule07()
113+
{
114+
var text = @"
115+
/*
116+
This is important stuff
117+
118+
Please keep this
119+
*/
120+
121+
using System;
122+
";
123+
var expected = @"
124+
/*
125+
This is important stuff
126+
127+
Please keep this
128+
*/
129+
130+
using System;
131+
";
132+
Verify(text, expected);
133+
}
134+
135+
[Fact]
136+
public void TestHasNoIllegalHeadersFormattingRule08()
137+
{
138+
var text = @"
139+
/*
140+
*/
141+
142+
using System;
143+
";
144+
var expected = @"
145+
/*
146+
*/
147+
148+
using System;
149+
";
150+
Verify(text, expected);
151+
}
152+
[Fact]
153+
public void TestHasNoIllegalHeadersFormattingRule09()
154+
{
155+
var text = @"/* <owner> foo </owner> */
156+
/* <owner> bar </owner> */
157+
/* <owner> baz </owner> */
158+
159+
using System;
160+
";
161+
var expected = @"
162+
using System;
163+
";
164+
Verify(text, expected);
165+
}
166+
167+
[Fact]
168+
public void TestHasNoIllegalHeadersFormattingRule10()
169+
{
170+
var text = @"/* <owner> bar </owner>
171+
<owner> baz </owner> */
172+
173+
using System;
174+
";
175+
var expected = @"
176+
using System;
177+
";
178+
Verify(text, expected);
179+
}
180+
181+
[Fact]
182+
public void TestHasNoIllegalHeadersFormattingRule11()
183+
{
184+
var text = @"// ==++== bar ==++==
185+
/* <owner> baz </owner> */
186+
187+
using System;
188+
";
189+
var expected = @"
190+
using System;
191+
";
192+
Verify(text, expected);
193+
}
194+
195+
[Fact]
196+
public void TestHasNoIllegalHeadersFormattingRule12()
197+
{
198+
var text = @"// ==++== bar ==++==
199+
/* <owner> baz </owner> */
200+
// ==++== bar ==++==
201+
/* <owner> baz </owner> */
202+
// <owner> baz </owner>
203+
204+
using System;
205+
";
206+
var expected = @"
207+
using System;
208+
";
209+
Verify(text, expected);
210+
}
211+
212+
[Fact]
213+
public void TestHasNoIllegalHeadersFormattingRule13()
214+
{
215+
var text = @"
216+
/* ============================
217+
<owner> foobar</owner>
218+
This is important
219+
============================ */
220+
221+
using System;
222+
";
223+
var expected = @"
224+
/* ============================
225+
This is important
226+
============================ */
227+
228+
using System;
229+
";
230+
Verify(text, expected);
231+
}
232+
233+
[Fact]
234+
public void TestHasNoIllegalHeadersFormattingRule14()
235+
{
236+
var text = @"
237+
/* ============================
238+
Test0.cs
239+
============================ */
240+
using System;
241+
";
242+
var expected = @"
243+
using System;
244+
";
245+
Verify(text, expected);
246+
}
247+
248+
internal override IFormattingRule GetFormattingRule()
249+
{
250+
return new Rules.HasNoIllegalHeadersFormattingRule();
251+
}
252+
}
253+
}

0 commit comments

Comments
 (0)