diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer.Test/Material/DoNotUseStringPropertyNamesTests.cs b/UnityEngineAnalyzer/UnityEngineAnalyzer.Test/Material/DoNotUseStringPropertyNamesTests.cs new file mode 100644 index 0000000..7867e5c --- /dev/null +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer.Test/Material/DoNotUseStringPropertyNamesTests.cs @@ -0,0 +1,86 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; +using NUnit.Framework; +using RoslynNUnitLight; +using UnityEngineAnalyzer.Material; + +namespace UnityEngineAnalyzer.Test.Material +{ + + [TestFixture] + sealed class DoNotUseStringPropertyNamesAnalyzerTests : AnalyzerTestFixture + { + protected override string LanguageName => LanguageNames.CSharp; + protected override DiagnosticAnalyzer CreateAnalyzer() => new DoNotUseStringPropertyNamesAnalyzer(); + + [Test] + public void MaterialGetFloatWithStringProperty() + { + const string code = @" +using UnityEngine; + +class C : MonoBehaviour +{ + Material material; + + void Start() + { + [|material.GetFloat(""_Shininess"")|]; + } +}"; + + Document document; + TextSpan span; + TestHelpers.TryGetDocumentAndSpanFromMarkup(code, LanguageName, MetadataReferenceHelper.UsingUnityEngine, out document, out span); + + HasDiagnostic(document, span, DiagnosticIDs.DoNotUseStringPropertyNamesInMaterial); + } + + [Test] + public void MaterialSetFloatWithStringProperty() + { + const string code = @" +using UnityEngine; + +class C : MonoBehaviour +{ + Material material; + + void Start() + { + [|material.SetFloat(""_Shininess"", 1.2f)|]; + } +}"; + + Document document; + TextSpan span; + TestHelpers.TryGetDocumentAndSpanFromMarkup(code, LanguageName, MetadataReferenceHelper.UsingUnityEngine, out document, out span); + + HasDiagnostic(document, span, DiagnosticIDs.DoNotUseStringPropertyNamesInMaterial); + } + + [Test] + public void MaterialSetMatrixWithStringProperty() + { + const string code = @" +using UnityEngine; + +class C : MonoBehaviour +{ + Material material; + + void Start() + { + [|material.SetVector(""_WaveAndDistance"", Vector3.one)|]; + } +}"; + + Document document; + TextSpan span; + TestHelpers.TryGetDocumentAndSpanFromMarkup(code, LanguageName, MetadataReferenceHelper.UsingUnityEngine, out document, out span); + + HasDiagnostic(document, span, DiagnosticIDs.DoNotUseStringPropertyNamesInMaterial); + } + } +} diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer.Test/UnityEngineAnalyzer.Test.csproj b/UnityEngineAnalyzer/UnityEngineAnalyzer.Test/UnityEngineAnalyzer.Test.csproj index cc86884..92ddb09 100644 --- a/UnityEngineAnalyzer/UnityEngineAnalyzer.Test/UnityEngineAnalyzer.Test.csproj +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer.Test/UnityEngineAnalyzer.Test.csproj @@ -135,6 +135,7 @@ + diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticDescriptors.cs b/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticDescriptors.cs index 0ac196e..5f71662 100644 --- a/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticDescriptors.cs +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticDescriptors.cs @@ -7,6 +7,7 @@ using UnityEngineAnalyzer.FindMethodsInUpdate; using UnityEngineAnalyzer.ForEachInUpdate; using UnityEngineAnalyzer.IL2CPP; +using UnityEngineAnalyzer.Material; using UnityEngineAnalyzer.OnGUI; using UnityEngineAnalyzer.StringMethods; @@ -147,5 +148,15 @@ static class DiagnosticDescriptors isEnabledByDefault: true, description: new LocalizableResourceString(nameof(DoNotUseStateNameResource.Description), DoNotUseStateNameResource.ResourceManager, typeof(DoNotUseStateNameResource)) ); + + public static readonly DiagnosticDescriptor DoNotUseStringPropertyNames = new DiagnosticDescriptor( + id: DiagnosticIDs.DoNotUseStringPropertyNamesInMaterial, + title: new LocalizableResourceString(nameof(DoNotUseStringPropertyNamesResource.Title), DoNotUseStringPropertyNamesResource.ResourceManager, typeof(DoNotUseStringPropertyNamesResource)), + messageFormat: new LocalizableResourceString(nameof(DoNotUseStringPropertyNamesResource.MessageFormat), DoNotUseStringPropertyNamesResource.ResourceManager, typeof(DoNotUseStringPropertyNamesResource)), + category: DiagnosticCategories.Performance, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: new LocalizableResourceString(nameof(DoNotUseStringPropertyNamesResource.Description), DoNotUseStringPropertyNamesResource.ResourceManager, typeof(DoNotUseStringPropertyNamesResource)) + ); } } diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticIDs.cs b/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticIDs.cs index 5d99f94..6e339ca 100644 --- a/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticIDs.cs +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer/DiagnosticIDs.cs @@ -13,6 +13,7 @@ public static class DiagnosticIDs public const string UnsealedDerivedClass = "UEA0008"; public const string InvokeFunctionMissing = "UEA0009"; public const string DoNotUseStateNameInAnimator = "UEA0010"; + public const string DoNotUseStringPropertyNamesInMaterial = "UEA0011"; //NOTES: These should probably be on their own analyzer - as they are not specific to Unity public const string DoNotUseRemoting = "AOT0001"; diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesAnalyzer.cs b/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesAnalyzer.cs new file mode 100644 index 0000000..b18cad7 --- /dev/null +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesAnalyzer.cs @@ -0,0 +1,75 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Immutable; + +namespace UnityEngineAnalyzer.Material +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class DoNotUseStringPropertyNamesAnalyzer : DiagnosticAnalyzer + { + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(DiagnosticDescriptors.DoNotUseStringPropertyNames); + + private static readonly ImmutableHashSet materialStringPropertyMethods = ImmutableHashSet.Create( + "GetColor", + "GetColorArray", + "GetFloat", + "GetFloatArray", + "GetInt", + "GetMatrix", + "GetMatrixArray", + "GetTexture", + "GetTextureOffset", + "GetTextureScale", + "GetVector", + "GetVectorArray", + "SetBuffer", + "SetColor", + "SetColorArray", + "SetFloat", + "SetFloatArray", + "SetInt", + "SetMatrix", + "SetMatrixArray", + "SetTexture", + "SetTextureOffset", + "SetTextureScale", + "SetVector", + "SetVectorArray"); + + public override void Initialize(AnalysisContext context) + { + context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.InvocationExpression); + } + + private static void AnalyzeNode(SyntaxNodeAnalysisContext context) + { + var invocation = context.Node as InvocationExpressionSyntax; + if (invocation == null) + { + return; + } + + var name = invocation.MethodName(); + + // check if any of the methods are used + if (!materialStringPropertyMethods.Contains(name)) { return; } + + var symbolInfo = context.SemanticModel.GetSymbolInfo(invocation); + var methodSymbol = symbolInfo.Symbol as IMethodSymbol; + + var containingClass = methodSymbol.ContainingType; + + // check if the method is the one from UnityEngine.Animator + if (containingClass.ContainingNamespace.Name.Equals("UnityEngine") && containingClass.Name.Equals("Material")) + { + if (methodSymbol.Parameters[0].Type.MetadataName == "String") + { + var diagnostic = Diagnostic.Create(DiagnosticDescriptors.DoNotUseStringPropertyNames, invocation.GetLocation(), containingClass.Name, methodSymbol.Name); + context.ReportDiagnostic(diagnostic); + } + } + } + } +} diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesResource.Designer.cs b/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesResource.Designer.cs new file mode 100644 index 0000000..7a95cd1 --- /dev/null +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesResource.Designer.cs @@ -0,0 +1,91 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by a tool. +// Runtime Version:4.0.30319.42000 +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + +namespace UnityEngineAnalyzer.Material { + using System; + using System.Reflection; + + + /// + /// A strongly-typed resource class, for looking up localized strings, etc. + /// + // This class was auto-generated by the StronglyTypedResourceBuilder + // class via a tool like ResGen or Visual Studio. + // To add or remove a member, edit your .ResX file then rerun ResGen + // with the /str option, or rebuild your VS project. + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + internal class DoNotUseStringPropertyNamesResource { + + private static global::System.Resources.ResourceManager resourceMan; + + private static global::System.Globalization.CultureInfo resourceCulture; + + [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + internal DoNotUseStringPropertyNamesResource() { + } + + /// + /// Returns the cached ResourceManager instance used by this class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Resources.ResourceManager ResourceManager { + get { + if (object.ReferenceEquals(resourceMan, null)) { + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("UnityEngineAnalyzer.Material.DoNotUseStringPropertyNamesResource", typeof(DoNotUseStringPropertyNamesResource).GetTypeInfo().Assembly); + resourceMan = temp; + } + return resourceMan; + } + } + + /// + /// Overrides the current thread's CurrentUICulture property for all + /// resource lookups using this strongly typed resource class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Globalization.CultureInfo Culture { + get { + return resourceCulture; + } + set { + resourceCulture = value; + } + } + + /// + /// Looks up a localized string similar to Integer comparison is faster than string comparisons, Use Shader.PropertyToID("name");. + /// + internal static string Description { + get { + return ResourceManager.GetString("Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use integer proerty ID instead. + /// + internal static string MessageFormat { + get { + return ResourceManager.GetString("MessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use integer proerty ID instead. + /// + internal static string Title { + get { + return ResourceManager.GetString("Title", resourceCulture); + } + } + } +} diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesResource.resx b/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesResource.resx new file mode 100644 index 0000000..c702a54 --- /dev/null +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer/Material/DoNotUseStringPropertyNamesResource.resx @@ -0,0 +1,132 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + Integer comparison is faster than string comparisons, Use Shader.PropertyToID("name"); + An optional longer localizable description of the diagnostic. + + + Use integer proerty ID instead + The format-able message the diagnostic displays. + + + Use integer proerty ID instead + The title of the diagnostic. + + \ No newline at end of file diff --git a/UnityEngineAnalyzer/UnityEngineAnalyzer/UnityEngineAnalyzer.csproj b/UnityEngineAnalyzer/UnityEngineAnalyzer/UnityEngineAnalyzer.csproj index bd0fcda..6fb8a0b 100644 --- a/UnityEngineAnalyzer/UnityEngineAnalyzer/UnityEngineAnalyzer.csproj +++ b/UnityEngineAnalyzer/UnityEngineAnalyzer/UnityEngineAnalyzer.csproj @@ -95,6 +95,12 @@ True UnsealedDerivedClassResources.resx + + + True + True + DoNotUseStringPropertyNamesResource.resx + @@ -158,6 +164,10 @@ ResXFileCodeGenerator UnsealedDerivedClassResources.Designer.cs + + ResXFileCodeGenerator + DoNotUseStringPropertyNamesResource.Designer.cs + ResXFileCodeGenerator DoNotUseOnGUIResources.Designer.cs