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

Commit 5ab1e54

Browse files
dsplaisteddsplaisted
committed
Apply suggestions from code review feedback
1 parent 7e08433 commit 5ab1e54

File tree

7 files changed

+98
-22
lines changed

7 files changed

+98
-22
lines changed

‎src/CodeFormatter/CodeFormatter.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@
115115
<Name>Microsoft.DotNet.CodeFormatting</Name>
116116
</ProjectReference>
117117
</ItemGroup>
118+
<ItemGroup>
119+
<Content Include="MSTestNamespaces.txt" />
120+
</ItemGroup>
118121
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
119122
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
120123
Other similar extension points exist, see Microsoft.Common.targets.
@@ -123,5 +126,6 @@
123126
<Target Name="AfterBuild">
124127
<Copy SourceFiles="$(ProjectDir)IllegalHeaders.md" DestinationFolder="$(OutDir)" ContinueOnError="true" />
125128
<Copy SourceFiles="$(ProjectDir)CopyrightHeader.md" DestinationFolder="$(OutDir)" ContinueOnError="true" />
129+
<Copy SourceFiles="$(ProjectDir)MSTestNamespaces.txt" DestinationFolder="$(OutDir)" ContinueOnError="true" />
126130
</Target>
127131
</Project>

‎src/CodeFormatter/MSTestNamespaces.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
N:Microsoft.VisualStudio.TestPlatform.UnitTestFramework
2+
N:Microsoft.Bcl.Testing
3+
N:Microsoft.VisualStudio.TestTools.UnitTesting
4+
N:CoreFXTestLibrary

‎src/CodeFormatter/Program.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
using System;
2-
using System.ComponentModel.Composition.Hosting;
2+
using System.Collections.Generic;
33
using System.IO;
44
using System.Linq;
55
using System.Threading;
66
using System.Threading.Tasks;
7-
87
using Microsoft.CodeAnalysis.MSBuild;
98
using Microsoft.DotNet.CodeFormatting;
10-
using System.Collections.Generic;
119

1210
namespace CodeFormatter
1311
{

‎src/Microsoft.DotNet.CodeFormatting/FormattingEngine.cs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,20 @@
55
using System.ComponentModel.Composition.Hosting;
66
using System.ComponentModel.Composition;
77
using System.Collections.Generic;
8+
using Microsoft.DotNet.CodeFormatting.Rules;
89

910
namespace Microsoft.DotNet.CodeFormatting
1011
{
1112
public static class FormattingEngine
1213
{
14+
private const string RuleTypeNotFoundError = "The specified rule type was not found: ";
15+
1316
public static IFormattingEngine Create(IEnumerable<string> ruleTypes)
1417
{
1518
var catalog = new AssemblyCatalog(typeof(FormattingEngine).Assembly);
1619

17-
var ruleTypesHash = new HashSet<string>(ruleTypes);
18-
if (ruleTypesHash.Count == 0)
19-
{
20-
ruleTypesHash.Add(RuleTypeConstants.FormatCodeRuleType);
21-
}
20+
var ruleTypesHash = new HashSet<string>(ruleTypes, StringComparer.InvariantCultureIgnoreCase);
21+
var notFoundRuleTypes = new HashSet<string>(ruleTypes, StringComparer.InvariantCultureIgnoreCase);
2222

2323
var filteredCatalog = new FilteredCatalog(catalog, cpd =>
2424
{
@@ -27,11 +27,15 @@ public static IFormattingEngine Create(IEnumerable<string> ruleTypes)
2727
em.ContractName == AttributedModelServices.GetContractName(typeof(IFormattingFilter))))
2828
{
2929
object ruleType;
30-
if (cpd.Metadata.TryGetValue("RuleType", out ruleType))
30+
if (cpd.Metadata.TryGetValue(RuleTypeConstants.PartMetadataKey, out ruleType))
3131
{
32-
if (ruleType is string && !ruleTypesHash.Contains((string)ruleType))
32+
if (ruleType is string)
3333
{
34-
return false;
34+
notFoundRuleTypes.Remove((string)ruleType);
35+
if (!ruleTypesHash.Contains((string)ruleType))
36+
{
37+
return false;
38+
}
3539
}
3640
}
3741
}
@@ -41,6 +45,13 @@ public static IFormattingEngine Create(IEnumerable<string> ruleTypes)
4145

4246
var container = new CompositionContainer(filteredCatalog);
4347
var engine = container.GetExportedValue<IFormattingEngine>();
48+
49+
// Need to do this after the catalog is queried, otherwise the lambda won't have been run
50+
foreach (var notFoundRuleType in notFoundRuleTypes)
51+
{
52+
(RuleTypeNotFoundError + notFoundRuleType).WriteConsoleError(1, "");
53+
}
54+
4455
return engine;
4556
}
4657
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@
100100
<Compile Include="Rules\IsFormattedFormattingRule.cs" />
101101
<Compile Include="Rules\IsSimplifiedFormattingRule.cs" />
102102
<Compile Include="Rules\RuleExtensions.cs" />
103-
<Compile Include="Rules\UsesXunitForTests.cs" />
103+
<Compile Include="Rules\UsesXunitForTestsFormattingRule.cs" />
104104
<Compile Include="RuleTypeConstants.cs" />
105105
</ItemGroup>
106106
<ItemGroup>

‎src/Microsoft.DotNet.CodeFormatting/RuleTypeConstants.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ namespace Microsoft.DotNet.CodeFormatting
1010
static class RuleTypeConstants
1111
{
1212
public const string PartMetadataKey = "RuleType";
13-
14-
public const string FormatCodeRuleType = "FormatCode";
1513
public const string ConvertTestsRuleType = "ConvertTests";
1614
}
1715
}

‎src/Microsoft.DotNet.CodeFormatting/Rules/UsesXunitForTests.cs renamed to ‎src/Microsoft.DotNet.CodeFormatting/Rules/UsesXunitForTestsFormattingRule.cs

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,31 @@
1111
using Microsoft.CodeAnalysis.CSharp;
1212
using Microsoft.CodeAnalysis.CodeGeneration;
1313
using System.Runtime.Serialization;
14+
using System.IO;
1415

1516
namespace Microsoft.DotNet.CodeFormatting.Rules
1617
{
17-
[RuleOrder(1)]
18+
[RuleOrder(101)]
1819
[PartMetadata(RuleTypeConstants.PartMetadataKey, RuleTypeConstants.ConvertTestsRuleType)]
19-
internal sealed class UsesXunitForTests : IFormattingRule
20+
internal sealed class UsesXunitForTestsFormattingRule : IFormattingRule
2021
{
22+
private static object _lockObject = new object();
23+
private static HashSet<string> _mstestNamespaces;
24+
25+
private const string FileNotFoundError = "The MSTestNamespaces.txt file was not found.";
26+
2127
public async Task<Document> ProcessAsync(Document document, CancellationToken cancellationToken)
2228
{
2329
var root = await document.GetSyntaxRootAsync(cancellationToken) as CompilationUnitSyntax;
2430

2531
if (root == null)
2632
return document;
2733

34+
if (!LoadMSTestNamespaces())
35+
{
36+
return document;
37+
}
38+
2839
var originalRoot = root;
2940

3041
SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
@@ -38,9 +49,7 @@ public async Task<Document> ProcessAsync(Document document, CancellationToken ca
3849
if (symbolInfo.Symbol != null)
3950
{
4051
string namespaceDocID = symbolInfo.Symbol.GetDocumentationCommentId();
41-
if (namespaceDocID == "N:Microsoft.VisualStudio.TestPlatform.UnitTestFramework" ||
42-
namespaceDocID == "N:Microsoft.Bcl.Testing" ||
43-
namespaceDocID == "N:Microsoft.VisualStudio.TestTools.UnitTesting")
52+
if (_mstestNamespaces.Contains(namespaceDocID))
4453
{
4554
needsChanges = true;
4655
}
@@ -111,7 +120,7 @@ private void RemoveTestClassAttributes(CompilationUnitSyntax root, SemanticModel
111120
if (typeInfo.Type != null)
112121
{
113122
string attributeTypeDocID = typeInfo.Type.GetDocumentationCommentId();
114-
if (attributeTypeDocID == "T:Microsoft.VisualStudio.TestTools.UnitTesting.TestClassAttribute")
123+
if (IsTestNamespaceType(attributeTypeDocID, "TestClassAttribute"))
115124
{
116125
return true;
117126

@@ -152,7 +161,7 @@ private void ChangeTestMethodAttributesToFact(CompilationUnitSyntax root, Semant
152161
if (typeInfo.Type != null)
153162
{
154163
string attributeTypeDocID = typeInfo.Type.GetDocumentationCommentId();
155-
if (attributeTypeDocID == "T:Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute")
164+
if (IsTestNamespaceType(attributeTypeDocID, "TestMethodAttribute"))
156165
{
157166
nodesToReplace.Add(attributeSyntax);
158167
}
@@ -194,7 +203,7 @@ private void ChangeAssertCalls(CompilationUnitSyntax root, SemanticModel semanti
194203
if (expressionTypeInfo.Type != null)
195204
{
196205
string expressionDocID = expressionTypeInfo.Type.GetDocumentationCommentId();
197-
if (expressionDocID == "T:Microsoft.VisualStudio.TestTools.UnitTesting.Assert")
206+
if (IsTestNamespaceType(expressionDocID, "Assert"))
198207
{
199208
string newMethodName;
200209
if (assertMethodsToRename.TryGetValue(methodCallSyntax.Name.Identifier.Text, out newMethodName))
@@ -252,6 +261,58 @@ private static SyntaxTriviaList RemoveCompilerDirectives(SyntaxTriviaList stl)
252261
}
253262

254263
return stl;
264+
}
265+
266+
private static bool IsTestNamespaceType(string docID, string simpleTypeName)
267+
{
268+
if (docID == null)
269+
{
270+
return false;
271+
}
272+
273+
int lastPeriod = docID.LastIndexOf('.');
274+
if (lastPeriod < 0)
275+
{
276+
return false;
277+
}
278+
279+
string simpleTypeNameFromDocID = docID.Substring(lastPeriod + 1);
280+
if (simpleTypeNameFromDocID != simpleTypeName)
281+
{
282+
return false;
283+
}
284+
285+
string namespaceDocID = "N" + docID.Substring(1, lastPeriod - 1);
286+
return _mstestNamespaces.Contains(namespaceDocID);
287+
}
288+
289+
private static bool LoadMSTestNamespaces()
290+
{
291+
lock(_lockObject)
292+
{
293+
if (_mstestNamespaces != null)
294+
{
295+
return true;
296+
}
297+
298+
var filePath = Path.Combine(
299+
Path.GetDirectoryName(Uri.UnescapeDataString(new UriBuilder(typeof(UsesXunitForTestsFormattingRule).Assembly.CodeBase).Path)),
300+
"MSTestNamespaces.txt");
301+
302+
if (!File.Exists(filePath))
303+
{
304+
FileNotFoundError.WriteConsoleError(1, "MSTestNamespaces.txt");
305+
return false;
306+
}
307+
308+
var lines = File.ReadAllLines(filePath);
309+
_mstestNamespaces = new HashSet<string>(lines);
310+
return true;
311+
}
312+
313+
314+
315+
255316
}
256317

257318
class TransformationTracker

0 commit comments

Comments
 (0)