From ac58f458b803966f492bb76e112785c9eaa67997 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Fri, 8 Jan 2016 06:02:37 -0600 Subject: [PATCH 1/5] Implement SA0002 (InvalidSettingsFile) Fixes #2029 --- .../DeserializationFailureBehavior.cs | 24 ++++++++ .../Settings/SettingsHelper.cs | 27 ++++++++- .../SpecialRules/SA0002InvalidSettingsFile.cs | 60 +++++++++++++++++++ .../SpecialRules/SpecialResources.Designer.cs | 29 +++++++++ .../SpecialRules/SpecialResources.resx | 11 ++++ .../StyleCop.Analyzers.csproj | 2 + StyleCopAnalyzers.sln | 1 + documentation/SA0002.md | 40 +++++++++++++ 8 files changed, 191 insertions(+), 3 deletions(-) create mode 100644 StyleCop.Analyzers/StyleCop.Analyzers/Settings/DeserializationFailureBehavior.cs create mode 100644 StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs create mode 100644 documentation/SA0002.md diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/DeserializationFailureBehavior.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/DeserializationFailureBehavior.cs new file mode 100644 index 000000000..8b6e376b9 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/DeserializationFailureBehavior.cs @@ -0,0 +1,24 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers +{ + using Newtonsoft.Json; + using StyleCop.Analyzers.Settings.ObjectModel; + + /// + /// Defines the behavior of various methods in the event of a deserialization error. + /// + internal enum DeserializationFailureBehavior + { + /// + /// When deserialization fails, return a default instance. + /// + ReturnDefaultSettings, + + /// + /// When deserialization fails, throw a containing details about the error. + /// + ThrowException + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/SettingsHelper.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/SettingsHelper.cs index 3aa789820..55ebd4d19 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Settings/SettingsHelper.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Settings/SettingsHelper.cs @@ -41,6 +41,10 @@ static SettingsHelper() /// /// Gets the StyleCop settings. /// + /// + /// If a occurs while deserializing the settings file, a default settings + /// instance is returned. + /// /// The context that will be used to determine the StyleCop settings. /// The cancellation token that the operation will observe. /// A instance that represents the StyleCop settings for the given context. @@ -52,15 +56,32 @@ internal static StyleCopSettings GetStyleCopSettings(this SyntaxTreeAnalysisCont /// /// Gets the StyleCop settings. /// + /// + /// If a occurs while deserializing the settings file, a default settings + /// instance is returned. + /// /// The analyzer options that will be used to determine the StyleCop settings. /// The cancellation token that the operation will observe. /// A instance that represents the StyleCop settings for the given context. internal static StyleCopSettings GetStyleCopSettings(this AnalyzerOptions options, CancellationToken cancellationToken) { - return GetStyleCopSettings(options != null ? options.AdditionalFiles : ImmutableArray.Create(), cancellationToken); + return GetStyleCopSettings(options, DeserializationFailureBehavior.ReturnDefaultSettings, cancellationToken); } - private static StyleCopSettings GetStyleCopSettings(ImmutableArray additionalFiles, CancellationToken cancellationToken) + /// + /// Gets the StyleCop settings. + /// + /// The analyzer options that will be used to determine the StyleCop settings. + /// The behavior of the method when a occurs while + /// deserializing the settings file. + /// The cancellation token that the operation will observe. + /// A instance that represents the StyleCop settings for the given context. + internal static StyleCopSettings GetStyleCopSettings(this AnalyzerOptions options, DeserializationFailureBehavior failureBehavior, CancellationToken cancellationToken) + { + return GetStyleCopSettings(options != null ? options.AdditionalFiles : ImmutableArray.Create(), failureBehavior, cancellationToken); + } + + private static StyleCopSettings GetStyleCopSettings(ImmutableArray additionalFiles, DeserializationFailureBehavior failureBehavior, CancellationToken cancellationToken) { try { @@ -74,7 +95,7 @@ private static StyleCopSettings GetStyleCopSettings(ImmutableArray return the default settings. } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs new file mode 100644 index 000000000..5f6784144 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs @@ -0,0 +1,60 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers.SpecialRules +{ + using System; + using System.Collections.Immutable; + using System.Globalization; + using Microsoft.CodeAnalysis; + using Microsoft.CodeAnalysis.Diagnostics; + using Newtonsoft.Json; + + /// + /// The stylecop.json settings file could not be loaded due to a deserialization failure. + /// + [NoCodeFix("No automatic code fix is possible for general JSON syntax errors.")] + [DiagnosticAnalyzer(LanguageNames.CSharp)] + internal class SA0002InvalidSettingsFile : DiagnosticAnalyzer + { + /// + /// The ID for diagnostics produced by the analyzer. + /// + public const string DiagnosticId = "SA0002"; + private const string HelpLink = "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA0002.md"; + private static readonly LocalizableString Title = new LocalizableResourceString(nameof(SpecialResources.SA0002Title), SpecialResources.ResourceManager, typeof(SpecialResources)); + private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(SpecialResources.SA0002MessageFormat), SpecialResources.ResourceManager, typeof(SpecialResources)); + private static readonly LocalizableString Description = new LocalizableResourceString(nameof(SpecialResources.SA0002Description), SpecialResources.ResourceManager, typeof(SpecialResources)); + + private static readonly DiagnosticDescriptor Descriptor = + new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.SpecialRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); + + private static readonly Action CompilationAction = HandleCompilation; + + /// + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(Descriptor); + + /// + public override void Initialize(AnalysisContext context) + { + context.RegisterCompilationAction(CompilationAction); + } + + private static void HandleCompilation(CompilationAnalysisContext context) + { + try + { + SettingsHelper.GetStyleCopSettings(context.Options, DeserializationFailureBehavior.ThrowException, context.CancellationToken); + } + catch (JsonException ex) + { + string details = ex.ToString(); + string completeDescription = string.Format(Description.ToString(CultureInfo.CurrentCulture), details); + + var completeDescriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.SpecialRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, completeDescription, HelpLink); + context.ReportDiagnostic(Diagnostic.Create(completeDescriptor, Location.None)); + } + } + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.Designer.cs b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.Designer.cs index 55c197f84..6acda0678 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.Designer.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.Designer.cs @@ -116,5 +116,34 @@ internal static string SA0001Title { return ResourceManager.GetString("SA0001Title", resourceCulture); } } + + /// + /// Looks up a localized string similar to Various errors in the stylecop.json file can prevent the file from being loaded by the analyzers. In this case, the default settings are used instead. + /// + ///{0}. + /// + internal static string SA0002Description { + get { + return ResourceManager.GetString("SA0002Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The stylecop.json settings file could not be loaded. + /// + internal static string SA0002MessageFormat { + get { + return ResourceManager.GetString("SA0002MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Invalid settings file. + /// + internal static string SA0002Title { + get { + return ResourceManager.GetString("SA0002Title", resourceCulture); + } + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.resx b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.resx index 3d4604877..67ac0ca0e 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.resx +++ b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SpecialResources.resx @@ -142,4 +142,15 @@ Note that some situations are not affected by the bug: XML comment analysis disabled + + Various errors in the stylecop.json file can prevent the file from being loaded by the analyzers. In this case, the default settings are used instead. + +{0} + + + The stylecop.json settings file could not be loaded + + + Invalid settings file + \ No newline at end of file diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj b/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj index 829855daf..66dd0f9e8 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj +++ b/StyleCop.Analyzers/StyleCop.Analyzers/StyleCop.Analyzers.csproj @@ -267,6 +267,7 @@ + @@ -323,6 +324,7 @@ + True True diff --git a/StyleCopAnalyzers.sln b/StyleCopAnalyzers.sln index 6fc6e0bf4..bd31f65d5 100644 --- a/StyleCopAnalyzers.sln +++ b/StyleCopAnalyzers.sln @@ -39,6 +39,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "documentation", "documentat documentation\KnownChanges.md = documentation\KnownChanges.md documentation\SA0000Roslyn7446Workaround.md = documentation\SA0000Roslyn7446Workaround.md documentation\SA0001.md = documentation\SA0001.md + documentation\SA0002.md = documentation\SA0002.md documentation\SA1000.md = documentation\SA1000.md documentation\SA1001.md = documentation\SA1001.md documentation\SA1002.md = documentation\SA1002.md diff --git a/documentation/SA0002.md b/documentation/SA0002.md new file mode 100644 index 000000000..8caf4f3d6 --- /dev/null +++ b/documentation/SA0002.md @@ -0,0 +1,40 @@ +# SA0002 + + + + + + + + + + + + + + +
TypeNameSA0002InvalidSettingsFile
CheckIdSA0002
CategorySpecial Rules
+ +:memo: This rule is new for StyleCop Analyzers, and was not present in StyleCop Classic. + +## Cause + +The stylecop.json settings file could not be loaded due to a deserialization error. + +## Rule description + +This rule reports cases where the StyleCop Analyzers settings file could not be loaded. When this occurs, the various +analyzers automatically fall back to using the default settings, which may not match the user's expectations. + +## How to fix violations + +To fix a violation of this rule, start by checking the following items: + +* Ensure stylecop.json contains valid JSON syntax. The file may be opened in Visual Studio 2015 to check for + common errors (they are reported by the IDE in the Errors window). +* Review the [configuration](Configuration.md) documentation and ensure the contents of stylecop.json contain + valid settings. + +## How to suppress violations + +This warning can only be suppressed by disabling the warning in the **ruleset** file for the project. From a4888af3e72f72a79fd5e5d25ccfc82e40d61e6f Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Fri, 8 Jan 2016 06:14:37 -0600 Subject: [PATCH 2/5] Implement unit tests for SA0002 --- .../Settings/SettingsFileCodeFixProvider.cs | 5 +- .../SpecialRules/SA0002UnitTests.cs | 72 +++++++++++++++++++ .../StyleCop.Analyzers.Test.csproj | 1 + 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Settings/SettingsFileCodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Settings/SettingsFileCodeFixProvider.cs index c2144aeea..1ab280466 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Settings/SettingsFileCodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Settings/SettingsFileCodeFixProvider.cs @@ -23,8 +23,7 @@ namespace StyleCop.Analyzers.Settings [Shared] internal class SettingsFileCodeFixProvider : CodeFixProvider { - private const string StyleCopSettingsFileName = "stylecop.json"; - private const string DefaultSettingsFileContent = @"{ + internal const string DefaultSettingsFileContent = @"{ // ACTION REQUIRED: This file was automatically added to your project, but it // will not take effect until additional steps are taken to enable it. See the // following page for additional information: @@ -40,6 +39,8 @@ internal class SettingsFileCodeFixProvider : CodeFixProvider } "; + private const string StyleCopSettingsFileName = "stylecop.json"; + /// public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create( diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs new file mode 100644 index 000000000..83413683d --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs @@ -0,0 +1,72 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +namespace StyleCop.Analyzers.Test.SpecialRules +{ + using System.Collections.Generic; + using System.Threading; + using System.Threading.Tasks; + using Analyzers.Settings; + using Analyzers.SpecialRules; + using Microsoft.CodeAnalysis.Diagnostics; + using TestHelper; + using Xunit; + + /// + /// Unit tests for . + /// + public class SA0002UnitTests : DiagnosticVerifier + { + private const string TestCode = @" +namespace NamespaceName { } +"; + + private string settings; + + [Fact] + public async Task TestMissingSettingsAsync() + { + await this.VerifyCSharpDiagnosticAsync(TestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestValidSettingsAsync() + { + this.settings = SettingsFileCodeFixProvider.DefaultSettingsFileContent; + + await this.VerifyCSharpDiagnosticAsync(TestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestInvalidSettingsAsync() + { + this.settings = @" +{ + ""$schema"": ""https://raw.githubusercontent.com/DotNetAnalyzers/StyleCopAnalyzers/master/StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json"" + ""settings"": { + ""documentationRules"": { + ""companyName"": ""ACME, Inc"", + ""copyrightText"": ""Copyright 2015 {companyName}. All rights reserved."" + } + } +} +"; + + DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(null, 0, 0); + + await this.VerifyCSharpDiagnosticAsync(TestCode, expected, CancellationToken.None).ConfigureAwait(false); + } + + /// + protected override string GetSettings() + { + return this.settings; + } + + /// + protected override IEnumerable GetCSharpDiagnosticAnalyzers() + { + yield return new SA0002InvalidSettingsFile(); + } + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj b/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj index 3f0ef2754..67918296c 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/StyleCop.Analyzers.Test.csproj @@ -360,6 +360,7 @@ + From cb9627453283b9b15b7d79adf43ade1f759c6780 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Fri, 8 Jan 2016 07:34:06 -0600 Subject: [PATCH 3/5] Fix test for diagnostic without a location --- .../SpecialRules/SA0002UnitTests.cs | 3 +- .../Verifiers/DiagnosticVerifier.cs | 29 ++++++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs index 83413683d..1957ea201 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0002UnitTests.cs @@ -52,7 +52,8 @@ public async Task TestInvalidSettingsAsync() } "; - DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(null, 0, 0); + // This diagnostic is reported without a location + DiagnosticResult expected = this.CSharpDiagnostic(); await this.VerifyCSharpDiagnosticAsync(TestCode, expected, CancellationToken.None).ConfigureAwait(false); } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/Verifiers/DiagnosticVerifier.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/Verifiers/DiagnosticVerifier.cs index b5cabc506..92076218c 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/Verifiers/DiagnosticVerifier.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/Verifiers/DiagnosticVerifier.cs @@ -326,6 +326,26 @@ private static string FormatDiagnostics(ImmutableArray analy return builder.ToString(); } + private static bool IsSubjectToExclusion(DiagnosticResult result) + { + if (result.Id.StartsWith("CS", StringComparison.Ordinal)) + { + return false; + } + + if (result.Id.StartsWith("AD", StringComparison.Ordinal)) + { + return false; + } + + if (result.Locations.Length == 0) + { + return false; + } + + return true; + } + /// /// General method that gets a collection of actual s found in the source after the /// analyzer is run, then verifies each of them. @@ -346,12 +366,13 @@ private async Task VerifyDiagnosticsAsync(string[] sources, string language, Imm if (filenames == null) { // Also check if the analyzer honors exclusions - if (expected.Any(x => x.Id.StartsWith("SA", StringComparison.Ordinal) || x.Id.StartsWith("SX", StringComparison.Ordinal))) + if (expected.Any(IsSubjectToExclusion)) { - // We want to look at non-stylecop diagnostics only. We also insert a new line at the beginning - // so we have to move all diagnostic location down by one line + // Diagnostics reported by the compiler and analyzer diagnostics which don't have a location will + // still be reported. We also insert a new line at the beginning so we have to move all diagnostic + // locations which have a specific position down by one line. var expectedResults = expected - .Where(x => !x.Id.StartsWith("SA", StringComparison.Ordinal) && !x.Id.StartsWith("SX", StringComparison.Ordinal)) + .Where(x => !IsSubjectToExclusion(x)) .Select(x => x.WithLineOffset(1)) .ToArray(); From e3bb82abc8c021cbdd690960093b69335ff2c4c8 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Sat, 9 Jan 2016 04:46:59 -0600 Subject: [PATCH 4/5] Update SA0001 unit test Previously the test used the wrong form for verifying diagnostics without a location. --- .../StyleCop.Analyzers.Test/SpecialRules/SA0001UnitTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0001UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0001UnitTests.cs index 4c4d3edb8..217a623d3 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0001UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/SpecialRules/SA0001UnitTests.cs @@ -42,7 +42,8 @@ public async Task TestDisabledDocumentationModesAsync(DocumentationMode document } "; - DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(null, 0, 0); + // This diagnostic is reported without a location + DiagnosticResult expected = this.CSharpDiagnostic(); this.documentationMode = documentationMode; await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false); From 41b5dc3ae3999645a494b1aa8ae4a0ab6659cc3a Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 11 Jan 2016 12:45:55 -0600 Subject: [PATCH 5/5] Show the message but not the stack trace in SA0002 details --- .../SpecialRules/SA0002InvalidSettingsFile.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs index 5f6784144..87cb7cc32 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/SpecialRules/SA0002InvalidSettingsFile.cs @@ -49,7 +49,7 @@ private static void HandleCompilation(CompilationAnalysisContext context) } catch (JsonException ex) { - string details = ex.ToString(); + string details = ex.Message; string completeDescription = string.Format(Description.ToString(CultureInfo.CurrentCulture), details); var completeDescriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.SpecialRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, completeDescription, HelpLink);