Skip to content

Commit

Permalink
Add PushProperty support for naming analyzer
Browse files Browse the repository at this point in the history
  • Loading branch information
olsh committed Dec 24, 2020
1 parent 8870134 commit 904ba0c
Show file tree
Hide file tree
Showing 21 changed files with 312 additions and 60 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Look for `Structured Logging` in Settings -> Plugins -> Browse repositories.
* [Template should be a compile-time constant](rules/TemplateIsNotCompileTimeConstantProblem.md)
* [Prefer named properties instead of positional ones](rules/PositionalPropertyUsedProblem.md)
* [Inconsistent log property naming](rules/InconsistentLogPropertyNaming.md)
* [Inconsistent log property naming in context](rules/InconsistentContextLogPropertyNaming.md)
* [Log event messages should be fragments, not sentences](rules/InconsistentLogPropertyNaming.md)

## Credits
Expand Down
11 changes: 11 additions & 0 deletions rules/InconsistentContextLogPropertyNaming.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#### Inconsistent log property naming in context (can be configured in the extension settings)

Noncompliant Code Examples:
```csharp
LogContext.PushProperty("property_name", 1);
```

Compliant Solution:
```csharp
LogContext.PushProperty("PropertyName", 1);
```
1 change: 1 addition & 0 deletions src/ReSharper.Structured.Logging.sln
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Rules", "Rules", "{D93C6901
..\rules\ComplexObjectInContextDestructuringProblem.md = ..\rules\ComplexObjectInContextDestructuringProblem.md
..\rules\ContextualLoggerProblem.md = ..\rules\ContextualLoggerProblem.md
..\rules\ExceptionPassedAsTemplateArgumentProblem.md = ..\rules\ExceptionPassedAsTemplateArgumentProblem.md
..\rules\InconsistentContextLogPropertyNaming.md = ..\rules\InconsistentContextLogPropertyNaming.md
..\rules\InconsistentLogPropertyNaming.md = ..\rules\InconsistentLogPropertyNaming.md
..\rules\LogMessageIsSentenceProblem.md = ..\rules\LogMessageIsSentenceProblem.md
..\rules\PositionalPropertyUsedProblem.md = ..\rules\PositionalPropertyUsedProblem.md
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Linq;
using System.Linq;
using JetBrains.Metadata.Reader.API;
using JetBrains.ReSharper.Feature.Services.Daemon;
using JetBrains.ReSharper.Psi;
Expand All @@ -24,36 +24,13 @@ protected override void Run(
return;
}

var templateArgumentIndex = templateArgument.IndexOf();
var exceptionType = element.PsiModule.GetPredefinedType().TryGetType(PredefinedType.EXCEPTION_FQN, NullableAnnotation.Unknown);
if (exceptionType == null)
{
return;
}

ICSharpArgument invalidExceptionArgument = null;
foreach (var argument in element.ArgumentList.Arguments)
{
var argumentType = argument.Value?.Type();
if (!(argumentType is IDeclaredType declaredType))
{
continue;
}

if (!declaredType.IsSubtypeOf(exceptionType))
{
continue;
}

if (templateArgumentIndex > argument.IndexOf())
{
return;
}

invalidExceptionArgument = argument;
break;
}

ICSharpArgument invalidExceptionArgument = FindInvalidExceptionArgument(element, templateArgument, exceptionType);
if (invalidExceptionArgument == null)
{
return;
Expand Down Expand Up @@ -96,5 +73,32 @@ protected override void Run(

consumer.AddHighlighting(new ExceptionPassedAsTemplateArgumentWarning(invalidExceptionArgument.GetDocumentRange()));
}

private ICSharpArgument FindInvalidExceptionArgument(IInvocationExpression invocationExpression, ICSharpArgument templateArgument, IDeclaredType exceptionType)
{
var templateArgumentIndex = templateArgument.IndexOf();
foreach (var argument in invocationExpression.ArgumentList.Arguments)
{
var argumentType = argument.Value?.Type();
if (!(argumentType is IDeclaredType declaredType))
{
continue;
}

if (!declaredType.IsSubtypeOf(exceptionType))
{
continue;
}

if (templateArgumentIndex > argument.IndexOf())
{
return null;
}

return argument;
}

return null;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,21 @@ protected override void Run(
IInvocationExpression element,
ElementProblemAnalyzerData data,
IHighlightingConsumer consumer)
{
var namingType = element.GetProject()
?.GetSolution()
.GetSettingsStore()
.GetValue(StructuredLoggingSettingsAccessor.PropertyNamingType)
?? PropertyNamingType.PascalCase;

CheckPropertiesInTemplate(element, consumer, namingType);
CheckPropertiesInContext(element, consumer, namingType);
}

private void CheckPropertiesInTemplate(
IInvocationExpression element,
IHighlightingConsumer consumer,
PropertyNamingType namingType)
{
var templateArgument = element.GetTemplateArgument();
var templateText = templateArgument?.TryGetTemplateText();
Expand All @@ -40,39 +55,68 @@ protected override void Run(
return;
}

var namingType = element.GetProject()
?.GetSolution()
.GetSettingsStore()
.GetValue(StructuredLoggingSettingsAccessor.PropertyNamingType)
?? PropertyNamingType.PascalCase;

foreach (var property in messageTemplate.NamedProperties)
{
if (string.IsNullOrEmpty(property.PropertyName))
{
continue;
}

string suggestedName = property.PropertyName;
switch (namingType)
{
case PropertyNamingType.PascalCase:
suggestedName = StringUtil.MakeUpperCamelCaseName(property.PropertyName);
break;
case PropertyNamingType.CamelCase:
suggestedName = StringUtil.MakeUpperCamelCaseName(property.PropertyName).Decapitalize();
break;
case PropertyNamingType.SnakeCase:
suggestedName = StringUtil.MakeUnderscoreCaseName(property.PropertyName);
break;
}

var suggestedName = GetSuggestedName(property.PropertyName, namingType);
if (string.Equals(suggestedName, property.PropertyName))
{
continue;
}

consumer.AddHighlighting(new InconsistentLogPropertyNamingWarning(templateArgument.GetTokenInformation(property), property, suggestedName));
consumer.AddHighlighting(
new InconsistentLogPropertyNamingWarning(templateArgument.GetTokenInformation(property), property,
suggestedName));
}
}

private void CheckPropertiesInContext(
IInvocationExpression element,
IHighlightingConsumer consumer,
PropertyNamingType namingType)
{
if (!element.IsSerilogContextPushPropertyMethod())
{
return;
}

if (element.ArgumentList.Arguments.Count < 1)
{
return;
}

var propertyArgument = element.ArgumentList.Arguments[0];
var propertyName = propertyArgument.Value.ConstantValue.Value as string;
if (string.IsNullOrEmpty(propertyName))
{
return;
}

var suggestedName = GetSuggestedName(propertyName, namingType);
if (string.Equals(propertyName, suggestedName))
{
return;
}

consumer.AddHighlighting(new InconsistentContextLogPropertyNamingWarning(propertyArgument, propertyName, suggestedName));
}

private string GetSuggestedName(string propertyName, PropertyNamingType namingType)
{
switch (namingType)
{
case PropertyNamingType.PascalCase:
return StringUtil.MakeUpperCamelCaseName(propertyName);
case PropertyNamingType.CamelCase:
return StringUtil.MakeUpperCamelCaseName(propertyName).Decapitalize();
case PropertyNamingType.SnakeCase:
return StringUtil.MakeUnderscoreCaseName(propertyName);
default:
return propertyName;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected override void Run(
if (argumentsCount < 0)
{
consumer.AddHighlighting(
new TemplateFormatStringUnexistingArgumentWarning(
new TemplateFormatStringNonExistingArgumentWarning(
templateArgument.GetTokenInformation(property).DocumentRange));
}
}
Expand All @@ -67,7 +67,7 @@ protected override void Run(
if (position >= argumentsCount)
{
consumer.AddHighlighting(
new TemplateFormatStringUnexistingArgumentWarning(
new TemplateFormatStringNonExistingArgumentWarning(
templateArgument.GetTokenInformation(property).DocumentRange));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
using JetBrains.DocumentModel;
using JetBrains.ReSharper.Feature.Services.Daemon;
using JetBrains.ReSharper.Psi.CSharp;
using JetBrains.ReSharper.Psi.CSharp.Tree;

using ReSharper.Structured.Logging.Settings;

namespace ReSharper.Structured.Logging.Highlighting
{
[RegisterConfigurableSeverity(
SeverityId,
null,
StructuredLoggingGroup.Id,
Message,
Message,
Severity.WARNING)]
[ConfigurableSeverityHighlighting(
SeverityId,
CSharpLanguage.Name,
OverlapResolve = OverlapResolveKind.WARNING,
ToolTipFormatString = Message)]
public class InconsistentContextLogPropertyNamingWarning : InconsistentLogPropertyNamingWarningBase, IHighlighting
{
private readonly string _propertyName;

public const string SeverityId = "InconsistentContextLogPropertyNaming";

public InconsistentContextLogPropertyNamingWarning(ICSharpArgument argument, string propertyName, string suggestedName)
{
_propertyName = propertyName;
Argument = argument;
SuggestedName = suggestedName;
}

public string ErrorStripeToolTip => ToolTip;

public ICSharpArgument Argument { get; }


public string SuggestedName { get; }

public string ToolTip => GetToolTipMessage(_propertyName, SuggestedName);

public DocumentRange CalculateRange()
{
return Argument.GetDocumentRange();
}

public bool IsValid()
{
return Argument.GetDocumentRange().IsValid();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ namespace ReSharper.Structured.Logging.Highlighting
CSharpLanguage.Name,
OverlapResolve = OverlapResolveKind.WARNING,
ToolTipFormatString = Message)]
public class InconsistentLogPropertyNamingWarning : IHighlighting
public class InconsistentLogPropertyNamingWarning : InconsistentLogPropertyNamingWarningBase, IHighlighting
{
private const string Message = "Property name '{0}' does not naming rules'. Suggested name is '{1}'.";

public const string SeverityId = "InconsistentLogPropertyNaming";

public InconsistentLogPropertyNamingWarning(
Expand All @@ -44,7 +42,7 @@ public InconsistentLogPropertyNamingWarning(

public string SuggestedName { get; }

public string ToolTip => $"Property name '{NamedProperty.PropertyName}' does not naming rules'. Suggested name is '{SuggestedName}'.";
public string ToolTip => GetToolTipMessage(NamedProperty.PropertyName, SuggestedName);

public DocumentRange CalculateRange()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace ReSharper.Structured.Logging.Highlighting
{
public class InconsistentLogPropertyNamingWarningBase
{
protected const string Message = "Property name does not match naming rules.";

protected string GetToolTipMessage(string propertyName, string suggestedName)
{
return $"Property name '{propertyName}' does not match naming rules'. Suggested name is '{suggestedName}'.";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ namespace ReSharper.Structured.Logging.Highlighting
CSharpLanguage.Name,
OverlapResolve = OverlapResolveKind.WARNING,
ToolTipFormatString = Message)]
public class TemplateFormatStringUnexistingArgumentWarning : IHighlighting
public class TemplateFormatStringNonExistingArgumentWarning : IHighlighting
{
private const string SeverityId = "TemplateFormatStringProblem";

private const string Message = "Non-existing argument in message template";

private readonly DocumentRange _documentRange;

public TemplateFormatStringUnexistingArgumentWarning(DocumentRange documentRange)
public TemplateFormatStringNonExistingArgumentWarning(DocumentRange documentRange)
{
_documentRange = documentRange;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using System;

using JetBrains.Annotations;
using JetBrains.Application.Progress;
using JetBrains.ProjectModel;
using JetBrains.ReSharper.Feature.Services.QuickFixes;
using JetBrains.ReSharper.Psi.CSharp;
using JetBrains.ReSharper.Psi.CSharp.Tree;
using JetBrains.ReSharper.Psi.ExtensionsAPI.Tree;
using JetBrains.ReSharper.Resources.Shell;
using JetBrains.TextControl;
using JetBrains.Util;

using ReSharper.Structured.Logging.Highlighting;

namespace ReSharper.Structured.Logging.QuickFixes
{
[QuickFix]
public class RenameContextLogPropertyFix : QuickFixBase
{
private readonly ICSharpArgument _argument;

private readonly string _suggestedName;

public RenameContextLogPropertyFix([NotNull] InconsistentContextLogPropertyNamingWarning error)
{
_suggestedName = error.SuggestedName;
_argument = error.Argument;
}

public override string Text => $"Rename property to '{_suggestedName}'";

public override bool IsAvailable(IUserDataHolder cache)
{
return _argument.IsValid();
}

protected override Action<ITextControl> ExecutePsiTransaction(ISolution solution, IProgressIndicator progress)
{
using (WriteLockCookie.Create())
{
// ReSharper disable once AssignNullToNotNullAttribute
var factory = CSharpElementFactory.GetInstance(_argument.Expression, false);

var expression = factory.CreateExpression($"\"{_suggestedName}\"");
ModificationUtil.ReplaceChild(_argument.Expression, expression);
}

return null;
}
}
}
Loading

0 comments on commit 904ba0c

Please sign in to comment.