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.

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

Move C# editorconfig diagnostic support API to CompilationOptions, instead of SyntaxTree #44331

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a9899b1
Move analyzer config diagnostic options to CompilationOptions
agocke May 17, 2020
7e5edc5
Allow CompilationOptions.WithSyntaxTreeOptionsProvider to take a null…
jasonmalinowski Jun 17, 2020
9da4451
Ensure that calling CompilationOptions.With* doesn't lose the SyntaxT…
jasonmalinowski Jun 17, 2020
4e780f3
Allow TryApplyChanges to handle changes to .editorconfig files withou…
jasonmalinowski Jun 17, 2020
118aace
Ensure Solution.WithProjectCompilationOptions keeps SyntaxTreeOptionP…
jasonmalinowski Jun 17, 2020
d8993a3
Adjust tests for .editorconfig no longer being on trees
jasonmalinowski Jun 17, 2020
cdf22e5
Fix the TestFieldsForEqualsAndGetHashCode test
jasonmalinowski Jun 17, 2020
20d825a
Update the WorkCoordinatorTests
jasonmalinowski Jun 17, 2020
332eebe
Delete tests that no longer work since the CompilationOptions is changed
jasonmalinowski Jun 17, 2020
2fea5f9
Change the Project_Reload expected values
jasonmalinowski Jun 18, 2020
2e21874
Fix up tests changed in merge
agocke Jun 18, 2020
808ac8f
Allow null instead
agocke Jun 21, 2020
3efdb4e
Fix formatting
agocke Jun 22, 2020
4a72867
Remove remnants of isGenerated internal code
agocke Jun 27, 2020
05351be
Merge remote-tracking branch 'upstream/master' into compilationoption…
agocke Jun 27, 2020
21b3745
Adjust test to reflect isGenerated changes
agocke Jun 27, 2020
e4630cc
Fix up some formatting issues
jasonmalinowski Jul 2, 2020
6322777
Merge branch 'master' into compilationoptions-analyzerconfig
chsienki Jul 9, 2020
59ca755
Renaming some identifiers and rewording comments for clarity
jasonmalinowski Jul 15, 2020
d9df348
Merge branch 'dotnet/master' into compilationoptions-analyzerconfig
jasonmalinowski Jul 15, 2020
433c2d9
Document the obsolete parameters are now obsolete, and Obsolete overl…
jasonmalinowski Jul 16, 2020
1577f7f
Completely obsolete the methods that take obsolete parameters
jasonmalinowski Jul 24, 2020
d804929
Merge remote-tracking branch 'dotnet/master' into compilationoptions-…
jasonmalinowski Jul 24, 2020
f5f3e1d
Fix whitespace
jasonmalinowski Jul 24, 2020
6dc3748
Fix up the PublicAPI.Unshipped.txt files and also one more analyzer w…
jasonmalinowski Jul 24, 2020
17bdda0
Include obsoletion message in the Obsolete attributes
jasonmalinowski Jul 24, 2020
d2616ff
Make all the obsoletions be explicitly error-causing
jasonmalinowski Jul 24, 2020
0c2abce
Simplify and inline local function
jasonmalinowski Jul 24, 2020
f227479
Merge branch 'dotnet/master' into compilationoptions-analyzerconfig
jasonmalinowski Aug 6, 2020
d83cd94
Switch the obsoletions back to warnings
jasonmalinowski Aug 6, 2020
3423b25
Move back compat overloads to the end of the file
jasonmalinowski Aug 6, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ public async Task AnalyzeAsync(
// Bail out if analyzer is suppressed on this file or project.
// NOTE: Normally, we would not require this check in the analyzer as the analyzer driver has this optimization.
// However, this is a special analyzer that is directly invoked by the analysis host (IDE), so we do this check here.
if (tree.DiagnosticOptions.TryGetValue(IDEDiagnosticIds.RemoveUnnecessarySuppressionDiagnosticId, out var severity) ||
ReportDiagnostic severity;
if (
#if !CODE_STYLE
compilationWithAnalyzers.Compilation.Options.SyntaxTreeOptionsProvider != null &&
compilationWithAnalyzers.Compilation.Options.SyntaxTreeOptionsProvider.TryGetDiagnosticValue(tree, IDEDiagnosticIds.RemoveUnnecessarySuppressionDiagnosticId, out severity) ||
#endif
jasonmalinowski marked this conversation as resolved.
Show resolved Hide resolved
compilationWithAnalyzers.Compilation.Options.SpecificDiagnosticOptions.TryGetValue(IDEDiagnosticIds.RemoveUnnecessarySuppressionDiagnosticId, out severity))
{
if (severity == ReportDiagnostic.Suppress)
Expand Down
13 changes: 11 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,17 @@ internal virtual Symbol? ContainingMemberOrLambda
/// </summary>
internal bool AreNullableAnnotationsEnabled(SyntaxTree syntaxTree, int position)
{
Syntax.NullableContextState context = ((CSharpSyntaxTree)syntaxTree).GetNullableContextState(position);
CSharpSyntaxTree csTree = (CSharpSyntaxTree)syntaxTree;
Syntax.NullableContextState context = csTree.GetNullableContextState(position);

return context.AnnotationsState switch
{
Syntax.NullableContextState.State.Enabled => true,
Syntax.NullableContextState.State.Disabled => false,
Syntax.NullableContextState.State.ExplicitlyRestored => GetGlobalAnnotationState(),
Syntax.NullableContextState.State.Unknown => AreNullableAnnotationsGloballyEnabled(),
Syntax.NullableContextState.State.Unknown =>
!csTree.IsGeneratedCode(this.Compilation.Options.SyntaxTreeOptionsProvider)
&& AreNullableAnnotationsGloballyEnabled(),
_ => throw ExceptionUtilities.UnexpectedValue(context.AnnotationsState)
};
}
Expand All @@ -253,6 +256,12 @@ internal bool AreNullableAnnotationsEnabled(SyntaxToken token)
return AreNullableAnnotationsEnabled(token.SyntaxTree, token.SpanStart);
}

internal bool IsGeneratedCode(SyntaxToken token)
{
var tree = (CSharpSyntaxTree)token.SyntaxTree!;
return tree.IsGeneratedCode(Compilation.Options.SyntaxTreeOptionsProvider);
}

internal virtual bool AreNullableAnnotationsGloballyEnabled()
{
RoslynDebug.Assert(Next is object);
Expand Down
6 changes: 5 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
}
else
{
LazyMissingNonNullTypesContextDiagnosticInfo.ReportNullableReferenceTypesIfNeeded(AreNullableAnnotationsEnabled(questionToken), questionToken.GetLocation(), diagnostics);
LazyMissingNonNullTypesContextDiagnosticInfo.ReportNullableReferenceTypesIfNeeded(
AreNullableAnnotationsEnabled(questionToken),
IsGeneratedCode(questionToken),
questionToken.GetLocation(),
diagnostics);
}
}
else if (isForOverride || AreNullableAnnotationsEnabled(constraintSyntax.ClassOrStructKeyword))
Expand Down
15 changes: 13 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -488,16 +488,27 @@ internal NamespaceOrTypeOrAliasSymbolWithAnnotations BindNamespaceOrTypeOrAliasS
void reportNullableReferenceTypesIfNeeded(SyntaxToken questionToken, TypeWithAnnotations typeArgument = default)
{
bool isNullableEnabled = AreNullableAnnotationsEnabled(questionToken);
bool isGeneratedCode = IsGeneratedCode(questionToken);
var location = questionToken.GetLocation();

// Inside a method body or other executable code, we can question IsValueType without causing cycles.
if (typeArgument.HasType && !ShouldCheckConstraints)
{
LazyMissingNonNullTypesContextDiagnosticInfo.AddAll(isNullableEnabled, typeArgument, location, diagnostics);
LazyMissingNonNullTypesContextDiagnosticInfo.AddAll(
isNullableEnabled,
isGeneratedCode,
typeArgument,
location,
diagnostics);
}
else
{
LazyMissingNonNullTypesContextDiagnosticInfo.ReportNullableReferenceTypesIfNeeded(isNullableEnabled, typeArgument, location, diagnostics);
LazyMissingNonNullTypesContextDiagnosticInfo.ReportNullableReferenceTypesIfNeeded(
isNullableEnabled,
isGeneratedCode,
typeArgument,
location,
diagnostics);
}
}

Expand Down
31 changes: 27 additions & 4 deletions src/Compilers/CSharp/Portable/CSharpCompilationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public CSharpCompilationOptions(
debugPlusMode: false,
xmlReferenceResolver: xmlReferenceResolver,
sourceReferenceResolver: sourceReferenceResolver,
syntaxTreeOptionsProvider: null,
metadataReferenceResolver: metadataReferenceResolver,
assemblyIdentityComparer: assemblyIdentityComparer,
strongNameProvider: strongNameProvider,
Expand Down Expand Up @@ -204,6 +205,7 @@ internal CSharpCompilationOptions(
bool debugPlusMode,
XmlReferenceResolver? xmlReferenceResolver,
SourceReferenceResolver? sourceReferenceResolver,
SyntaxTreeOptionsProvider? syntaxTreeOptionsProvider,
MetadataReferenceResolver? metadataReferenceResolver,
AssemblyIdentityComparer? assemblyIdentityComparer,
StrongNameProvider? strongNameProvider,
Expand All @@ -216,8 +218,8 @@ internal CSharpCompilationOptions(
cryptoKeyContainer, cryptoKeyFile, cryptoPublicKey, delaySign, publicSign, optimizationLevel, checkOverflow,
platform, generalDiagnosticOption, warningLevel, specificDiagnosticOptions.ToImmutableDictionaryOrEmpty(),
concurrentBuild, deterministic, currentLocalTime, debugPlusMode, xmlReferenceResolver,
sourceReferenceResolver, metadataReferenceResolver, assemblyIdentityComparer,
strongNameProvider, metadataImportOptions, referencesSupersedeLowerVersions)
sourceReferenceResolver, syntaxTreeOptionsProvider, metadataReferenceResolver,
assemblyIdentityComparer, strongNameProvider, metadataImportOptions, referencesSupersedeLowerVersions)
{
this.Usings = usings.AsImmutableOrEmpty();
this.AllowUnsafe = allowUnsafe;
Expand Down Expand Up @@ -248,6 +250,7 @@ private CSharpCompilationOptions(CSharpCompilationOptions other) : this(
debugPlusMode: other.DebugPlusMode,
xmlReferenceResolver: other.XmlReferenceResolver,
sourceReferenceResolver: other.SourceReferenceResolver,
syntaxTreeOptionsProvider: other.SyntaxTreeOptionsProvider,
metadataReferenceResolver: other.MetadataReferenceResolver,
assemblyIdentityComparer: other.AssemblyIdentityComparer,
strongNameProvider: other.StrongNameProvider,
Expand Down Expand Up @@ -573,6 +576,16 @@ internal CSharpCompilationOptions WithReferencesSupersedeLowerVersions(bool valu
return new CSharpCompilationOptions(this) { SourceReferenceResolver = resolver };
}

public new CSharpCompilationOptions WithSyntaxTreeOptionsProvider(SyntaxTreeOptionsProvider? provider)
{
if (ReferenceEquals(provider, this.SyntaxTreeOptionsProvider))
{
return this;
}

return new CSharpCompilationOptions(this) { SyntaxTreeOptionsProvider = provider };
}

public new CSharpCompilationOptions WithMetadataReferenceResolver(MetadataReferenceResolver? resolver)
{
if (ReferenceEquals(resolver, this.MetadataReferenceResolver))
Expand Down Expand Up @@ -625,6 +638,9 @@ protected override CompilationOptions CommonWithXmlReferenceResolver(XmlReferenc
protected override CompilationOptions CommonWithSourceReferenceResolver(SourceReferenceResolver? resolver) =>
WithSourceReferenceResolver(resolver);

protected override CompilationOptions CommonWithSyntaxTreeOptionsProvider(SyntaxTreeOptionsProvider? provider)
=> WithSyntaxTreeOptionsProvider(provider);

protected override CompilationOptions CommonWithMetadataReferenceResolver(MetadataReferenceResolver? resolver) =>
WithMetadataReferenceResolver(resolver);

Expand Down Expand Up @@ -739,9 +755,15 @@ public override int GetHashCode()
Hash.Combine(TopLevelBinderFlags.GetHashCode(), this.NullableContextOptions.GetHashCode()))));
}

internal override Diagnostic FilterDiagnostic(Diagnostic diagnostic)
internal override Diagnostic? FilterDiagnostic(Diagnostic diagnostic)
{
return CSharpDiagnosticFilter.Filter(diagnostic, WarningLevel, NullableContextOptions, GeneralDiagnosticOption, SpecificDiagnosticOptions);
return CSharpDiagnosticFilter.Filter(
diagnostic,
WarningLevel,
NullableContextOptions,
GeneralDiagnosticOption,
SpecificDiagnosticOptions,
SyntaxTreeOptionsProvider);
}

protected override CompilationOptions CommonWithModuleName(string? moduleName)
Expand Down Expand Up @@ -901,6 +923,7 @@ public CSharpCompilationOptions(
debugPlusMode: false,
xmlReferenceResolver: xmlReferenceResolver,
sourceReferenceResolver: sourceReferenceResolver,
syntaxTreeOptionsProvider: null,
metadataReferenceResolver: metadataReferenceResolver,
assemblyIdentityComparer: assemblyIdentityComparer,
strongNameProvider: strongNameProvider,
Expand Down
80 changes: 29 additions & 51 deletions src/Compilers/CSharp/Portable/CommandLine/CSharpCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable enable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
Expand Down Expand Up @@ -36,7 +38,7 @@ protected CSharpCompiler(CSharpCommandLineParser parser, string responseFile, st
public override DiagnosticFormatter DiagnosticFormatter { get { return _diagnosticFormatter; } }
protected internal new CSharpCommandLineArguments Arguments { get { return (CSharpCommandLineArguments)base.Arguments; } }

public override Compilation CreateCompilation(
public override Compilation? CreateCompilation(
TextWriter consoleOutput,
TouchedFileLogger touchedFilesLogger,
ErrorLogger errorLogger,
Expand All @@ -51,7 +53,7 @@ public override Compilation CreateCompilation(
bool hadErrors = false;

var sourceFiles = Arguments.SourceFiles;
var trees = new SyntaxTree[sourceFiles.Length];
var trees = new SyntaxTree?[sourceFiles.Length];
var normalizedFilePaths = new string[sourceFiles.Length];
var diagnosticBag = DiagnosticBag.GetInstance();

Expand All @@ -65,9 +67,6 @@ public override Compilation CreateCompilation(
trees[i] = ParseFile(
parseOptions,
scriptParseOptions,
analyzerConfigOptions.IsDefault
? (AnalyzerConfigOptionsResult?)null
: analyzerConfigOptions[i],
ref hadErrors,
sourceFiles[i],
diagnosticBag,
Expand All @@ -87,9 +86,6 @@ public override Compilation CreateCompilation(
trees[i] = ParseFile(
parseOptions,
scriptParseOptions,
analyzerConfigOptions.IsDefault
? (AnalyzerConfigOptionsResult?)null
: analyzerConfigOptions[i],
ref hadErrors,
sourceFiles[i],
diagnosticBag,
Expand Down Expand Up @@ -163,23 +159,24 @@ public override Compilation CreateCompilation(
}

var loggingFileSystem = new LoggingStrongNameFileSystem(touchedFilesLogger, _tempDirectory);
var optionsProvider = new CompilerSyntaxTreeOptionsProvider(trees, analyzerConfigOptions);

return CSharpCompilation.Create(
Arguments.CompilationName,
trees.WhereNotNull(),
resolvedReferences,
Arguments.CompilationOptions.
WithMetadataReferenceResolver(referenceDirectiveResolver).
WithAssemblyIdentityComparer(assemblyIdentityComparer).
WithXmlReferenceResolver(xmlFileResolver).
WithStrongNameProvider(Arguments.GetStrongNameProvider(loggingFileSystem)).
WithSourceReferenceResolver(sourceFileResolver));
Arguments.CompilationOptions
.WithMetadataReferenceResolver(referenceDirectiveResolver)
.WithAssemblyIdentityComparer(assemblyIdentityComparer)
.WithXmlReferenceResolver(xmlFileResolver)
.WithStrongNameProvider(Arguments.GetStrongNameProvider(loggingFileSystem))
.WithSourceReferenceResolver(sourceFileResolver)
.WithSyntaxTreeOptionsProvider(optionsProvider));
}

private SyntaxTree ParseFile(
private SyntaxTree? ParseFile(
CSharpParseOptions parseOptions,
CSharpParseOptions scriptParseOptions,
AnalyzerConfigOptionsResult? analyzerConfigOptionsResult,
ref bool addedDiagnostics,
CommandLineSourceFile file,
DiagnosticBag diagnostics,
Expand All @@ -201,36 +198,20 @@ private SyntaxTree ParseFile(
else
{
Debug.Assert(fileDiagnostics.Count == 0);
return ParseFile(parseOptions, scriptParseOptions, content, file, analyzerConfigOptionsResult);
return ParseFile(parseOptions, scriptParseOptions, content, file);
}
}

private static SyntaxTree ParseFile(
CSharpParseOptions parseOptions,
CSharpParseOptions scriptParseOptions,
SourceText content,
CommandLineSourceFile file,
AnalyzerConfigOptionsResult? analyzerConfigOptionsResult)
CommandLineSourceFile file)
{
ImmutableDictionary<string, ReportDiagnostic> diagnosticOptions;
bool? isUserConfiguredGeneratedCode;
if (analyzerConfigOptionsResult.HasValue)
{
diagnosticOptions = analyzerConfigOptionsResult.Value.TreeOptions;
isUserConfiguredGeneratedCode = GeneratedCodeUtilities.GetIsGeneratedCodeFromOptions(analyzerConfigOptionsResult.Value.AnalyzerOptions);
}
else
{
diagnosticOptions = null;
isUserConfiguredGeneratedCode = null;
}

var tree = SyntaxFactory.ParseSyntaxTree(
content,
file.IsScript ? scriptParseOptions : parseOptions,
file.Path,
diagnosticOptions,
Copy link
Member

Choose a reason for hiding this comment

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

So any existing analyzers out there will just break if they were using the obsolete properties? I'm not terribly worried about it as the users are likely rare (or non-existent) but just wanted to confirm the intent since this will go through API review.

Copy link
Member Author

@agocke agocke May 19, 2020

Choose a reason for hiding this comment

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

The intended breaking changes are:

If you added options to an analyzer config, they will not get propagated to the syntax trees. You can still access the property on the syntax tree but it is obsolete, and none of the options from the analyzer config files will be present.

If you added options to the trees directly, those will be preserved.

Eventually, we can consider removing the obsolete APIs all together.

isUserConfiguredGeneratedCode);
file.Path);

// prepopulate line tables.
// we will need line tables anyways and it is better to not wait until we are in emit
Expand Down Expand Up @@ -264,26 +245,23 @@ protected override string GetOutputFileName(Compilation compilation, Cancellatio

var comp = (CSharpCompilation)compilation;

Symbol entryPoint = comp.ScriptClass;
if ((object)entryPoint == null)
Symbol? entryPoint = comp.ScriptClass;
if (entryPoint is null)
{
var method = comp.GetEntryPoint(cancellationToken);
if ((object)method != null)
if (method is object)
{
entryPoint = method.PartialImplementationPart ?? method;
}
else
{
// no entrypoint found - an error will be reported and the compilation won't be emitted
return "error";
}
}

if ((object)entryPoint != null)
{
string entryPointFileName = PathUtilities.GetFileName(entryPoint.Locations.First().SourceTree.FilePath);
return Path.ChangeExtension(entryPointFileName, ".exe");
}
else
{
// no entrypoint found - an error will be reported and the compilation won't be emitted
return "error";
}
string entryPointFileName = PathUtilities.GetFileName(entryPoint.Locations.First().SourceTree!.FilePath);
return Path.ChangeExtension(entryPointFileName, ".exe");
}
else
{
Expand Down Expand Up @@ -312,7 +290,7 @@ public override void PrintLangVersions(TextWriter consoleOutput)
consoleOutput.WriteLine(ErrorFacts.GetMessage(MessageID.IDS_LangVersions, Culture));
var defaultVersion = LanguageVersion.Default.MapSpecifiedToEffectiveVersion();
var latestVersion = LanguageVersion.Latest.MapSpecifiedToEffectiveVersion();
foreach (LanguageVersion v in Enum.GetValues(typeof(LanguageVersion)))
foreach (var v in (LanguageVersion[])Enum.GetValues(typeof(LanguageVersion)))
{
if (v == defaultVersion)
{
Expand Down Expand Up @@ -376,13 +354,13 @@ protected override void ResolveEmbeddedFilesFromExternalSourceDirectives(
foreach (LineDirectiveTriviaSyntax directive in tree.GetRoot().GetDirectives(
d => d.IsActive && !d.HasErrors && d.Kind() == SyntaxKind.LineDirectiveTrivia))
{
string path = (string)directive.File.Value;
var path = (string?)directive.File.Value;
if (path == null)
{
continue;
}

string resolvedPath = resolver.ResolveReference(path, tree.FilePath);
string? resolvedPath = resolver.ResolveReference(path, tree.FilePath);
if (resolvedPath == null)
{
diagnostics.Add(
Expand Down
Loading