Skip to content

Commit

Permalink
Merge pull request #1545 from riganti/fix/tobrowserlocaltime-issues
Browse files Browse the repository at this point in the history
Added `UnsupportedCallSiteAttribute` and an analyzer
  • Loading branch information
acizmarik authored Feb 1, 2023
2 parents f02cfc5 + 0b9055c commit a7a2f9f
Show file tree
Hide file tree
Showing 14 changed files with 284 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
using System.Threading.Tasks;
using DotVVM.Analyzers.ApiUsage;
using Xunit;
using VerifyCS = DotVVM.Analyzers.Tests.CSharpAnalyzerVerifier<
DotVVM.Analyzers.ApiUsage.UnsupportedCallSiteAttributeAnalyzer>;

namespace DotVVM.Analyzers.Tests.ApiUsage
{
public class UnsupportedCallSiteAttributeTests
{
[Fact]
public async Task Test_NoDiagnostics_InvokeMethod_WithoutUnsupportedCallSiteAttribute()
{
var test = @"
using System;
using System.IO;
namespace ConsoleApplication1
{
public class RegularClass
{
public void Target()
{
}
public void CallSite()
{
Target();
}
}
}";

await VerifyCS.VerifyAnalyzerAsync(test);
}

[Fact]
public async Task Test_Warning_InvokeMethod_WithUnsupportedCallSiteAttribute()
{
await VerifyCS.VerifyAnalyzerAsync(@"
using System;
using System.IO;
using DotVVM.Framework.CodeAnalysis;
namespace ConsoleApplication1
{
public class RegularClass
{
[UnsupportedCallSite(CallSiteType.ServerSide)]
public void Target()
{
}
public void CallSite()
{
{|#0:Target()|};
}
}
}",

VerifyCS.Diagnostic(UnsupportedCallSiteAttributeAnalyzer.DoNotInvokeMethodFromUnsupportedCallSite)
.WithLocation(0).WithArguments("Target", string.Empty));
}

[Fact]
public async Task Test_Warning_InvokeMethod_WithUnsupportedCallSiteAttribute_WithReason()
{
await VerifyCS.VerifyAnalyzerAsync(@"
using System;
using System.IO;
using DotVVM.Framework.CodeAnalysis;
namespace ConsoleApplication1
{
public class RegularClass
{
[UnsupportedCallSite(CallSiteType.ServerSide, ""REASON"")]
public void Target()
{
}
public void CallSite()
{
{|#0:Target()|};
}
}
}",

VerifyCS.Diagnostic(UnsupportedCallSiteAttributeAnalyzer.DoNotInvokeMethodFromUnsupportedCallSite)
.WithLocation(0).WithArguments("Target", "due to: \"REASON\""));
}
}
}
1 change: 1 addition & 0 deletions src/Analyzers/Analyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ Rule ID | Category | Severity | Notes
--------|----------|----------|-------
DotVVM02 | Serializability | Warning | ViewModelSerializabilityAnalyzer
DotVVM03 | Serializability | Warning | ViewModelSerializabilityAnalyzer
DotVVM04 | ApiUsage | Warning | UnsupportedCallSiteAttributeAnalyzer
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace DotVVM.Analyzers.ApiUsage
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UnsupportedCallSiteAttributeAnalyzer : DiagnosticAnalyzer
{
private static readonly LocalizableResourceString unsupportedCallSiteTitle = new(nameof(Resources.ApiUsage_UnsupportedCallSite_Title), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableResourceString unsupportedCallSiteMessage = new(nameof(Resources.ApiUsage_UnsupportedCallSite_Message), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableResourceString unsupportedCallSiteDescription = new(nameof(Resources.ApiUsage_UnsupportedCallSite_Description), Resources.ResourceManager, typeof(Resources));
private const string unsupportedCallSiteAttributeMetadataName = "DotVVM.Framework.CodeAnalysis.UnsupportedCallSiteAttribute";
private const int callSiteTypeServerUnderlyingValue = 0;

public static DiagnosticDescriptor DoNotInvokeMethodFromUnsupportedCallSite = new(
DotvvmDiagnosticIds.DoNotInvokeMethodFromUnsupportedCallSiteRuleId,
unsupportedCallSiteTitle,
unsupportedCallSiteMessage,
DiagnosticCategory.ApiUsage,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
unsupportedCallSiteDescription);

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

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.EnableConcurrentExecution();

context.RegisterOperationAction(context =>
{
var unsupportedCallSiteAttribute = context.Compilation.GetTypeByMetadataName(unsupportedCallSiteAttributeMetadataName);
if (unsupportedCallSiteAttribute is null)
return;

if (context.Operation is IInvocationOperation invocation)
{
var method = invocation.TargetMethod;
var attribute = method.GetAttributes().FirstOrDefault(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, unsupportedCallSiteAttribute));
if (attribute is null || !attribute.ConstructorArguments.Any())
return;

if (attribute.ConstructorArguments.First().Value is not int callSiteType || callSiteTypeServerUnderlyingValue != callSiteType)
return;

var reason = (string?)attribute.ConstructorArguments.Skip(1).First().Value;
context.ReportDiagnostic(
Diagnostic.Create(
DoNotInvokeMethodFromUnsupportedCallSite,
invocation.Syntax.GetLocation(),
invocation.TargetMethod.Name,
(reason != null) ? $"due to: \"{reason}\"" : string.Empty));
}
}, OperationKind.Invocation);
}
}
}
1 change: 1 addition & 0 deletions src/Analyzers/Analyzers/DiagnosticCategory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ internal static class DiagnosticCategory
{
public const string Serializability = nameof(Serializability);
public const string StaticCommands = nameof(StaticCommands);
public const string ApiUsage = nameof(ApiUsage);
}
}
1 change: 1 addition & 0 deletions src/Analyzers/Analyzers/DotvvmDiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ public static class DotvvmDiagnosticIds

public const string UseSerializablePropertiesInViewModelRuleId = "DotVVM02";
public const string DoNotUseFieldsInViewModelRuleId = "DotVVM03";
public const string DoNotInvokeMethodFromUnsupportedCallSiteRuleId = "DotVVM04";
}
}
27 changes: 27 additions & 0 deletions src/Analyzers/Analyzers/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/Analyzers/Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,15 @@
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="ApiUsage_UnsupportedCallSite_Description" xml:space="preserve">
<value>Method that declares that it should not be called on server is only meant to be invoked on client.</value>
</data>
<data name="ApiUsage_UnsupportedCallSite_Message" xml:space="preserve">
<value>Method '{0}' invocation is not supported on server {1}</value>
</data>
<data name="ApiUsage_UnsupportedCallSite_Title" xml:space="preserve">
<value>Unsupported call site</value>
</data>
<data name="Serializability_DoNotUseFields_Description" xml:space="preserve">
<value>Fields are not supported in viewmodels. Use properties to save state of viewmodels instead.</value>
</data>
Expand Down
8 changes: 8 additions & 0 deletions src/Framework/Core/CodeAnalysis/CallSiteType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace DotVVM.Framework.CodeAnalysis
{
public enum CallSiteType
{
ServerSide,
ClientSide
}
}
17 changes: 17 additions & 0 deletions src/Framework/Core/CodeAnalysis/UnsupportedCallSiteAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;

namespace DotVVM.Framework.CodeAnalysis
{
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, AllowMultiple = false)]
public class UnsupportedCallSiteAttribute : Attribute
{
public readonly CallSiteType Type;
public readonly string? Reason;

public UnsupportedCallSiteAttribute(CallSiteType type, string? reason = null)
{
Type = type;
Reason = reason;
}
}
}
1 change: 1 addition & 0 deletions src/Framework/Core/DotVVM.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<Description>This package contains base classes and interfaces of DotVVM that might be useful in a business layer.
DotVVM is an open source ASP.NET-based framework which allows to build modern web apps without writing any JavaScript code.</Description>
<Nullable>enable</Nullable>
<RootNamespace>DotVVM.Framework</RootNamespace>
</PropertyGroup>
<ItemGroup>
<EmbeddedResource Include="compiler\resources\**\*" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using DotVVM.Framework.CodeAnalysis;

namespace DotVVM.Framework.Binding.HelperNamespace
{
Expand All @@ -7,16 +8,14 @@ public static class DateTimeExtensions

/// <summary>
/// Converts the date (assuming it is in UTC) to browser's local time.
/// CAUTION: When evaluated on the server, no conversion is made as we don't know the browser timezone.
/// </summary>
[Obsolete("When evaluated on the server, no conversion is made as we don't know the browser timezone.")]
[UnsupportedCallSite(CallSiteType.ServerSide, "When evaluated on the server, no conversion is made as we don't know the browser timezone.")]
public static DateTime ToBrowserLocalTime(this DateTime value) => value;

/// <summary>
/// Converts the date (assuming it is in UTC) to browser's local time.
/// CAUTION: When evaluated on the server, no conversion is made as we don't know the browser timezone.
/// </summary>
[Obsolete("When evaluated on the server, no conversion is made as we don't know the browser timezone.")]
[UnsupportedCallSite(CallSiteType.ServerSide, "When evaluated on the server, no conversion is made as we don't know the browser timezone.")]
public static DateTime? ToBrowserLocalTime(this DateTime? value) => value;

}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using System;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using DotVVM.Framework.CodeAnalysis;
using DotVVM.Framework.Binding.Expressions;
using DotVVM.Framework.Compilation.ControlTree.Resolved;

namespace DotVVM.Framework.Compilation.ControlTree
{
public class UnsupportedCallSiteCheckingVisitor : ResolvedControlTreeVisitor
{
class ExpressionInspectingVisitor : ExpressionVisitor
{
public event Action<MethodInfo>? InvalidCallSiteDetected;

protected override Expression VisitMethodCall(MethodCallExpression node)
{
if (node.Method.IsDefined(typeof(UnsupportedCallSiteAttribute)))
{
var callSiteAttr = node.Method.CustomAttributes.FirstOrDefault(a => a.AttributeType == typeof(UnsupportedCallSiteAttribute))!;
if (callSiteAttr.ConstructorArguments.Any() && callSiteAttr.ConstructorArguments.First().Value is int type && type == (int)CallSiteType.ServerSide)
InvalidCallSiteDetected?.Invoke(node.Method);
}

return base.VisitMethodCall(node);
}
}

public override void VisitBinding(ResolvedBinding binding)
{
base.VisitBinding(binding);
if (binding.Binding is not ResourceBindingExpression and not CommandBindingExpression)
return;

var expressionVisitor = new ExpressionInspectingVisitor();
expressionVisitor.InvalidCallSiteDetected += method =>
binding.DothtmlNode?.AddWarning($"Evaluation of method \"{method.Name}\" on server-side may yield unexpected results.");

expressionVisitor.Visit(binding.Expression);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ public static IServiceCollection RegisterDotVVMServices(IServiceCollection servi
o.TreeVisitors.Add(() => ActivatorUtilities.CreateInstance<DataContextPropertyAssigningVisitor>(s));
o.TreeVisitors.Add(() => new UsedPropertiesFindingVisitor());
o.TreeVisitors.Add(() => new LifecycleRequirementsAssigningVisitor());
o.TreeVisitors.Add(() => new UnsupportedCallSiteCheckingVisitor());
});

services.TryAddSingleton<IDotvvmCacheAdapter, DefaultDotvvmCacheAdapter>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,21 @@ @viewModel System.Boolean
Assert.AreEqual("HTML attribute name 'Visble' should not contain uppercase letters. Did you mean Visible, or another DotVVM property?", attribute3.AttributeNameNode.NodeWarnings.Single());
}

[TestMethod]
public void DefaultViewCompiler_UnsupportedCallSite_ResourceBinding_Warning()
{
var markup = @"
@viewModel System.DateTime
{{resource: _this.ToBrowserLocalTime()}}
";
var literal = ParseSource(markup)
.Content.SelectRecursively(c => c.Content)
.Single(c => c.Metadata.Type == typeof(Literal));

Assert.AreEqual(1, literal.DothtmlNode.NodeWarnings.Count());
Assert.AreEqual("Evaluation of method \"ToBrowserLocalTime\" on server-side may yield unexpected results.", literal.DothtmlNode.NodeWarnings.First());
}

[TestMethod]
public void DefaultViewCompiler_DifferentControlPrimaryName()
{
Expand Down

0 comments on commit a7a2f9f

Please sign in to comment.