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

[WIP] resx analyzer #3566

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -6,6 +6,7 @@ RS0042 | RoslynDiagnosticsReliability | Warning | DoNotCopyValue
RS0043 | RoslynDiagnosticsMaintainability | Warning | DoNotCallGetTestAccessor
RS0044 | RoslynDiagnosticsMaintainability | Hidden | CreateTestAccessor
RS0045 | RoslynDiagnosticsMaintainability | Hidden | ExposeMemberForTesting
RS0047 | RoslynDiagnosticsDesign | Info | DefineResourceEntryCorrectly
Copy link
Member Author

Choose a reason for hiding this comment

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

#3352 is taking RS0046


### Changed Rules
Rule ID | New Category | New Severity | Old Category | Old Severity | Notes
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.IO;
using System.Text.RegularExpressions;
using System.Xml;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Text;

namespace Roslyn.Diagnostics.Analyzers
{
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class DefineResourceEntryCorrectly : DiagnosticAnalyzer
{
private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DefineResourceEntryCorrectlyTitle), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DefineResourceEntryCorrectlyMessage), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.DefineResourceEntryCorrectlyDescription), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));

internal static DiagnosticDescriptor Rule = new DiagnosticDescriptor(
RoslynDiagnosticIds.DefineResourceEntryCorrectlyRuleId,
s_localizableTitle,
s_localizableMessage,
DiagnosticCategory.RoslynDiagnosticsDesign,
DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: s_localizableDescription,
helpLinkUri: null,
customTags: WellKnownDiagnosticTags.Telemetry);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

private static readonly Regex s_dataNameRegex = new Regex("<data.*?name=\"(.*?)\"", RegexOptions.Compiled);
private static readonly Regex s_valueRegex = new Regex("<value>(.*?)</value>", RegexOptions.Compiled);
Comment on lines +35 to +36
Copy link
Member Author

Choose a reason for hiding this comment

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

It's never 100% clear to me when Compiled is better.

Comment on lines +35 to +36
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI .*? allows a non-greedy version of .* this prevents unwanting characters! Besides, it's usually better in term of performances because it reduces the backtracking.

private const StringComparison s_ResxContentStringComparison = StringComparison.Ordinal;

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.RegisterCompilationAction(context =>
{
foreach (var file in context.Options.AdditionalFiles)
{
context.CancellationToken.ThrowIfCancellationRequested();

var fileExtension = Path.GetExtension(file.Path);
if (!fileExtension.Equals(".resx", StringComparison.OrdinalIgnoreCase))
{
continue;
}

var isDescription = false;
var sourceText = file.GetText(context.CancellationToken);
Copy link
Member Author

Choose a reason for hiding this comment

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

I have tried a couple of resx/xml loader but we either don't have the right dll or this is failing because of the first line of the resx which doesn't seem to be xml compliant. Let me know if you know a better way that doing line by line reading.

foreach (var line in sourceText.Lines)
{
var text = line.ToString().Trim();

var match = s_dataNameRegex.Match(text);
Copy link
Member Author

Choose a reason for hiding this comment

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

If we go for this line by line check, do we want to have a StartsWith check before the regex match?

if (match.Success)
{
isDescription = match.Groups[1].Value.EndsWith("Description", s_ResxContentStringComparison);
continue;
}

match = s_valueRegex.Match(text);
if (match.Success)
{
var endsWithPeriod = match.Groups[1].Value.EndsWith(".", s_ResxContentStringComparison);

if (endsWithPeriod && !isDescription)
{
var linePositionSpan = sourceText.Lines.GetLinePositionSpan(line.Span);
var location = Location.Create(file.Path, line.Span, linePositionSpan);
Copy link
Member Author

@Evangelink Evangelink Apr 28, 2020

Choose a reason for hiding this comment

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

Where do we want to report? Full line? Only the .?

context.ReportDiagnostic(Diagnostic.Create(Rule, location));
}
else if (isDescription && !endsWithPeriod)
{
var linePositionSpan = sourceText.Lines.GetLinePositionSpan(line.Span);
var location = Location.Create(file.Path, line.Span, linePositionSpan);
Copy link
Member Author

Choose a reason for hiding this comment

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

Where do we want to report? Full line? The last character of the sentence?

context.ReportDiagnostic(Diagnostic.Create(Rule, location));
}
else
{
// do nothing
}
}
}
}
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,6 @@ internal static class RoslynDiagnosticIds
public const string DoNotCallGetTestAccessorRuleId = "RS0043";
public const string CreateTestAccessorRuleId = "RS0044";
public const string ExposeMemberForTestingRuleId = "RS0045";
public const string DefineResourceEntryCorrectlyRuleId = "RS0047";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -339,4 +339,13 @@
<data name="ExposeMemberForTestingTitle" xml:space="preserve">
<value>Expose member for testing</value>
</data>
<data name="DefineResourceEntryCorrectlyDescription" xml:space="preserve">
<value>Define resource entry correctly.</value>
</data>
<data name="DefineResourceEntryCorrectlyMessage" xml:space="preserve">
<value>Define resource entry correctly</value>
</data>
<data name="DefineResourceEntryCorrectlyTitle" xml:space="preserve">
<value>Define resource entry correctly</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Typy, které se dají nastavit jako výchozí, by měly mít pole, která se tak dají nastavit taky</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() je pomocná metoda vyhrazená pro testování. Produkční kód tento člen nesmí volat.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Auf den Standard festlegbare Typen müssen auf den Standard festlegbare Felder aufweisen.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() ist eine für Tests reservierte Hilfsmethode. Der Produktionscode darf diesen Member nicht aufrufen.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Los tipos defaultable deben tener campos defaultable</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() es un método auxiliar reservado para las pruebas. El código de producción no debe llamar a este miembro.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Les types par défaut doivent avoir des champs par défaut</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() est une méthode d'assistance réservée aux tests. Le code de production ne doit pas appeler ce membre.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">I tipi defaultable devono avere campi defaultable</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() è un metodo helper riservato per il test. IL codice di produzione non deve chiamare questo membro.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">既定に設定可能な型には、既定に設定可能なフィールドが必要です</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor () は、テスト用に予約されているヘルパー メソッドです。運用コードでこのメンバーを呼び出さないでください。</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">기본값 설정 가능 형식에는 기본값 설정 가능 필드가 있어야 함</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor()는 테스트용으로 예약된 도우미 메서드입니다. 프로덕션 코드는 이 멤버를 호출해서는 안 됩니다.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Typy dopuszczające wartość domyślną powinny mieć pola z wartościami domyślnymi</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() to metoda pomocnicza zarezerwowana do testowania. Kod produkcyjny nie może wywoływać tego elementu członkowskiego.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Os tipos usados como padrão devem ter campos usados como padrão</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() é um método auxiliar reservado para testes. O código de produção não deve chamar este membro.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Типы, допускающие значения по умолчанию, должны иметь соответствующие поля</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor() — это вспомогательный метод, используемый только для тестирования. Не вызывайте этот метод из рабочего кода.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,21 @@
<target state="translated">Varsayılan değer atanabilir türler varsayılan değer atanabilir alanlara sahip olmalıdır</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyDescription">
<source>Define resource entry correctly.</source>
<target state="new">Define resource entry correctly.</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyMessage">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DefineResourceEntryCorrectlyTitle">
<source>Define resource entry correctly</source>
<target state="new">Define resource entry correctly</target>
<note />
</trans-unit>
<trans-unit id="DoNotCallGetTestAccessorDescription">
<source>'GetTestAccessor()' is a helper method reserved for testing. Production code must not call this member.</source>
<target state="needs-review-translation">GetTestAccessor(), test için ayrılmış bir yardımcı metottur. Üretim kodu bu üyeyi çağırmamalıdır.</target>
Expand Down
Loading