Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure generated trees have a full path #60095

Closed
wants to merge 4 commits into from
Closed

Ensure generated trees have a full path #60095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4430ab0
Ensure generated trees have a full path in PDBs
chsienki Mar 10, 2022
87026ff
Add path map test
chsienki Mar 10, 2022
e6ac0bf
Add sarif tests
chsienki Mar 10, 2022
7aa7174
Extract path to local
chsienki Mar 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 120 additions & 2 deletions src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2451,13 +2451,14 @@ private static void ValidateEmbeddedSources_Windows(Dictionary<string, string> e
continue;
}

var sourceStr = Encoding.UTF8.GetString(sourceBlob.Array, sourceBlob.Offset, sourceBlob.Count);
// offset by 3 to skip the BOM
Copy link
Member

Choose a reason for hiding this comment

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

Odd to do this unconditionally. What if there is no BOM?

var sourceStr = Encoding.UTF8.GetString(sourceBlob.Array, sourceBlob.Offset + 3, sourceBlob.Count - 3);

Assert.Equal(expectedEmbeddedMap[docPath], sourceStr);
Assert.True(expectedEmbeddedMap.Remove(docPath));
}
}
catch
finally
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out we were silently ignoring failures, but this function was not actually being used anyway :/

{
symReader?.Dispose();
}
Expand Down Expand Up @@ -14324,6 +14325,123 @@ public void TestDuplicateAdditionalFiles(string additionalFilePath1, string addi
[InlineData("abc/a.txt", "./../ABC/a.txt", 2)]
public void TestDuplicateAdditionalFiles_Linux(string additionalFilePath1, string additionalFilePath2, int expectedCount) => TestDuplicateAdditionalFiles(additionalFilePath1, additionalFilePath2, expectedCount);


Copy link
Member

Choose a reason for hiding this comment

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

How does this behavior interact with [CallerFilePath] optional arguments? Now that the source file has a full path would expect some interaction with this feature.

[Theory]
[InlineData("/debug:embedded")]
[InlineData("/debug:portable")]
[InlineData("/debug:full")]
public void Generated_Trees_Have_Full_Path(string debugSwitch)
{
var dir = Temp.CreateDirectory();
var src = dir.CreateFile("temp.cs").WriteAllText("class C {}");
var generatedDir = dir.CreateDirectory("generated");

var generatedSource = "public class D { }";
var generator = new SingleFileTestGenerator(generatedSource, "generatedSource.cs");

VerifyOutput(dir, src, includeCurrentAssemblyAsAnalyzerReference: false, additionalFlags: new[] { "/generatedfilesout:" + generatedDir.Path, debugSwitch, "/out:embed.exe" }, generators: new[] { generator }, analyzers: null);

var generatorPrefix = GeneratorDriver.GetFilePathPrefixForGenerator(generator);

ValidateWrittenSources(new() { { Path.Combine(generatedDir.Path, generatorPrefix), new() { { "generatedSource.cs", generatedSource } } } });

Dictionary<string, string> expectedEmbeddedMap = new() { { Path.Combine(generatedDir.Path, generatorPrefix, "generatedSource.cs"), generatedSource } };
switch (debugSwitch)
{
case "/debug:embedded":
ValidateEmbeddedSources_Portable(expectedEmbeddedMap, dir, isEmbeddedPdb: true);
break;
case "/debug:portable":
ValidateEmbeddedSources_Portable(expectedEmbeddedMap, dir, isEmbeddedPdb: false);
break;
case "/debug:full":
ValidateEmbeddedSources_Windows(expectedEmbeddedMap, dir);
break;
}

// Clean up temp files
CleanupAllGeneratedFiles(src.Path);
}

[Theory]
[InlineData("/debug:embedded")]
[InlineData("/debug:portable")]
[InlineData("/debug:full")]
public void Generated_Trees_Have_Output_Path_When_No_GeneratedFilesOut(string debugSwitch)
{
var dir = Temp.CreateDirectory();
var src = dir.CreateFile("temp.cs").WriteAllText("class C {}");
var generatedDir = dir.CreateDirectory("generated");

var generatedSource = "public class D { }";
var generator = new SingleFileTestGenerator(generatedSource, "generatedSource.cs");

VerifyOutput(dir, src, includeCurrentAssemblyAsAnalyzerReference: false, additionalFlags: new[] { debugSwitch, "/out:embed.exe" }, generators: new[] { generator }, analyzers: null);

var generatorPrefix = GeneratorDriver.GetFilePathPrefixForGenerator(generator);

// validate that *no* sources were written
Assert.Empty(Directory.GetDirectories(generatedDir.Path));

// but we still have full paths (using output dir) in the PDBs
Copy link
Member

@cston cston Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Should we verify the files were written to the expected directory?

ValidateWrittenSources(new() { { generatedDir.Path, new() { { "generatedSource.cs", generatedSource } } } });
``` #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we're not actually emitting any files here, which we check that on line 14384.

Dictionary<string, string> expectedEmbeddedMap = new() { { Path.Combine(dir.Path, generatorPrefix, "generatedSource.cs"), generatedSource } };
switch (debugSwitch)
{
case "/debug:embedded":
ValidateEmbeddedSources_Portable(expectedEmbeddedMap, dir, isEmbeddedPdb: true);
break;
case "/debug:portable":
ValidateEmbeddedSources_Portable(expectedEmbeddedMap, dir, isEmbeddedPdb: false);
break;
case "/debug:full":
ValidateEmbeddedSources_Windows(expectedEmbeddedMap, dir);
break;
}

// Clean up temp files
CleanupAllGeneratedFiles(src.Path);
}

[Theory]
[InlineData("/debug:embedded")]
[InlineData("/debug:portable")]
[InlineData("/debug:full")]
public void Generated_Trees_Output_Path_Respects_PathMap(string debugSwitch)
{
var dir = Temp.CreateDirectory();
var src = dir.CreateFile("temp.cs").WriteAllText("class C {}");
var generatedDir = dir.CreateDirectory("generated");

var generatedSource = "public class D { }";
var generator = new SingleFileTestGenerator(generatedSource, "generatedSource.cs");

var mappedPath = Path.Combine("mapped", "path");

VerifyOutput(dir, src, includeCurrentAssemblyAsAnalyzerReference: false, additionalFlags: new[] { debugSwitch, "/out:embed.exe", $"/pathmap:{dir.Path}={mappedPath}" }, generators: new[] { generator }, analyzers: null);

var generatorPrefix = GeneratorDriver.GetFilePathPrefixForGenerator(generator);

// validate that *no* sources were written
Assert.Empty(Directory.GetDirectories(generatedDir.Path));

// but we still have full paths (using output dir) in the PDBs
Copy link
Member

@cston cston Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Should we verify the files were written to the expected directory?

ValidateWrittenSources(new() { { Path.Combine(mappedPath, generatorPrefix), new() { { "generatedSource.cs", generatedSource } } } });
``` #Closed

Dictionary<string, string> expectedEmbeddedMap = new() { { Path.Combine(mappedPath, generatorPrefix, "generatedSource.cs"), generatedSource } };
switch (debugSwitch)
{
case "/debug:embedded":
ValidateEmbeddedSources_Portable(expectedEmbeddedMap, dir, isEmbeddedPdb: true);
break;
case "/debug:portable":
ValidateEmbeddedSources_Portable(expectedEmbeddedMap, dir, isEmbeddedPdb: false);
break;
case "/debug:full":
ValidateEmbeddedSources_Windows(expectedEmbeddedMap, dir);
break;
}

// Clean up temp files
CleanupAllGeneratedFiles(src.Path);
}
}

[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
Expand Down
33 changes: 33 additions & 0 deletions src/Compilers/CSharp/Test/CommandLine/SarifErrorLoggerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.IO;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Diagnostics;
using Roslyn.Test.Utilities.TestGenerators;
using Xunit;
using static Microsoft.CodeAnalysis.CommonDiagnosticAnalyzers;
using static Roslyn.Test.Utilities.SharedResourceHelpers;
Expand All @@ -23,6 +24,7 @@ public abstract class SarifErrorLoggerTests : CommandLineTestBase
internal abstract string GetExpectedOutputForSimpleCompilerDiagnosticsSuppressed(CommonCompiler cmd, string sourceFile);
internal abstract string GetExpectedOutputForAnalyzerDiagnosticsWithAndWithoutLocation(MockCSharpCompiler cmd);
internal abstract string GetExpectedOutputForAnalyzerDiagnosticsWithSuppression(MockCSharpCompiler cmd, string justification);
internal abstract string GetExpectedOutputForSourceGenerator(CommonCompiler cmd, string sourceFile);

protected void NoDiagnosticsImpl()
{
Expand Down Expand Up @@ -300,5 +302,36 @@ class C
CleanupAllGeneratedFiles(sourceFile);
CleanupAllGeneratedFiles(errorLogFile);
}

protected void SourceGeneratorImpl()
{
var source = "public class C { }";
var dir = Temp.CreateDirectory();
var sourceFile = dir.CreateFile("C.cs").WriteAllText(source).Path;
var generatedDir = dir.CreateDirectory("generated");
var errorLogFile = Path.Combine(dir.Path, "ErrorLog.txt");
var generator = new SingleFileTestGenerator("publiiic class D { }", "D.generated.cs");
string[] arguments = new[] { "/nologo", sourceFile, "/preferreduilang:en", $"/errorlog:{errorLogFile}{ErrorLogQualifier}", "/generatedfilesout:" + generatedDir.Path };

var cmd = CreateCSharpCompiler(null, sourceFile, arguments, generators: ImmutableArray.Create<ISourceGenerator>(generator));

var outWriter = new StringWriter(CultureInfo.InvariantCulture);

var exitCode = cmd.Run(outWriter);
var actualConsoleOutput = outWriter.ToString().Trim();

Assert.Contains("CS0116", actualConsoleOutput);
Assert.Contains("CS5001", actualConsoleOutput);
Assert.NotEqual(0, exitCode);

var actualOutput = File.ReadAllText(errorLogFile).Trim();
var prefix = GeneratorDriver.GetFilePathPrefixForGenerator(generator);
var expectedOutput = GetExpectedOutputForSourceGenerator(cmd, Path.Combine(generatedDir.Path, prefix, "D.generated.cs"));

Assert.Equal(expectedOutput, actualOutput);

CleanupAllGeneratedFiles(sourceFile);
CleanupAllGeneratedFiles(errorLogFile);
}
}
}
71 changes: 71 additions & 0 deletions src/Compilers/CSharp/Test/CommandLine/SarifV1ErrorLoggerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,77 @@ internal override string GetExpectedOutputForSimpleCompilerDiagnosticsSuppressed
return expectedHeader + expectedIssues;
}

[ConditionalFact(typeof(WindowsOnly), Reason = "https://github.com/dotnet/roslyn/issues/30289")]
public void SourceGenerator()
{
SourceGeneratorImpl();
}

internal override string GetExpectedOutputForSourceGenerator(CommonCompiler cmd, string sourceFile)
{
var expectedHeader = GetExpectedErrorLogHeader(cmd);
var expectedIssues = string.Format(@"
""results"": [
{{
""ruleId"": ""CS0116"",
""level"": ""error"",
""message"": ""A namespace cannot directly contain members such as fields, methods or statements"",
""locations"": [
{{
""resultFile"": {{
""uri"": ""{0}"",
""region"": {{
""startLine"": 1,
""startColumn"": 1,
""endLine"": 1,
""endColumn"": 9
}}
}}
}}
]
}},
{{
""ruleId"": ""CS5001"",
""level"": ""error"",
""message"": ""Program does not contain a static 'Main' method suitable for an entry point""
}}
],
""rules"": {{
""CS0116"": {{
""id"": ""CS0116"",
""defaultLevel"": ""error"",
""helpUri"": ""https://msdn.microsoft.com/query/roslyn.query?appId=roslyn&k=k(CS0116)"",
""properties"": {{
""category"": ""Compiler"",
""isEnabledByDefault"": true,
""tags"": [
""Compiler"",
""Telemetry"",
""NotConfigurable""
]
}}
}},
""CS5001"": {{
""id"": ""CS5001"",
""defaultLevel"": ""error"",
""helpUri"": ""https://msdn.microsoft.com/query/roslyn.query?appId=roslyn&k=k(CS5001)"",
""properties"": {{
""category"": ""Compiler"",
""isEnabledByDefault"": true,
""tags"": [
""Compiler"",
""Telemetry"",
""NotConfigurable""
]
}}
}}
}}
}}
]
}}", AnalyzerForErrorLogTest.GetUriForPath(sourceFile));
return expectedHeader + expectedIssues;
}

[ConditionalFact(typeof(WindowsOnly), Reason = "https://github.com/dotnet/roslyn/issues/30289")]
public void SimpleCompilerDiagnosticsSuppressed()
{
Expand Down
97 changes: 97 additions & 0 deletions src/Compilers/CSharp/Test/CommandLine/SarifV2ErrorLoggerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,103 @@ public void AnalyzerDiagnosticsSuppressedWithNullJustification()
AnalyzerDiagnosticsSuppressedWithNullJustificationImpl();
}

[ConditionalFact(typeof(WindowsOnly))]
public void SourceGenerator()
{
SourceGeneratorImpl();
}

internal override string GetExpectedOutputForSourceGenerator(CommonCompiler cmd, string sourceFile)
{
var expectedOutput = @"{{
""$schema"": ""http://json.schemastore.org/sarif-2.1.0"",
""version"": ""2.1.0"",
""runs"": [
{{
""results"": [
{{
""ruleId"": ""CS0116"",
""ruleIndex"": 0,
""level"": ""error"",
""message"": {{
""text"": ""A namespace cannot directly contain members such as fields, methods or statements""
}},
""locations"": [
{{
""physicalLocation"": {{
""artifactLocation"": {{
""uri"": ""{5}""
}},
""region"": {{
""startLine"": 1,
""startColumn"": 1,
""endLine"": 1,
""endColumn"": 9
}}
}}
}}
]
}},
{{
""ruleId"": ""CS5001"",
""ruleIndex"": 1,
""level"": ""error"",
""message"": {{
""text"": ""Program does not contain a static 'Main' method suitable for an entry point""
}}
}}
],
""tool"": {{
""driver"": {{
""name"": ""{0}"",
""version"": ""{1}"",
""dottedQuadFileVersion"": ""{2}"",
""semanticVersion"": ""{3}"",
""language"": ""{4}"",
""rules"": [
{{
""id"": ""CS0116"",
""defaultConfiguration"": {{
""level"": ""error""
}},
""helpUri"": ""https://msdn.microsoft.com/query/roslyn.query?appId=roslyn&k=k(CS0116)"",
""properties"": {{
""category"": ""Compiler"",
""tags"": [
""Compiler"",
""Telemetry"",
""NotConfigurable""
]
}}
}},
{{
""id"": ""CS5001"",
""defaultConfiguration"": {{
""level"": ""error""
}},
""helpUri"": ""https://msdn.microsoft.com/query/roslyn.query?appId=roslyn&k=k(CS5001)"",
""properties"": {{
""category"": ""Compiler"",
""tags"": [
""Compiler"",
""Telemetry"",
""NotConfigurable""
]
}}
}}
]
}}
}},
""columnKind"": ""utf16CodeUnits""
}}
]
}}";
return FormatOutputText(
expectedOutput,
cmd,
AnalyzerForErrorLogTest.GetUriForPath(sourceFile));
}

private string FormatOutputText(
string s,
CommonCompiler compiler,
Expand Down
Loading