Skip to content

Map range attribute to value range annotation #1673

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

Merged
merged 3 commits into from
May 28, 2020
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Since 2018.1, the version numbers and release cycle match Rider's versions and r

### Added

- Use `Range` attribute to provide hints to integer dataflow analysis ([#1673](https://github.com/JetBrains/resharper-unity/pull/1673))
- Rider: Add "pausepoints" a type of breakpoint that doesn't suspend code execution, but pauses the Unity editor ([#1272](https://github.com/JetBrains/resharper-unity/issues/1272), [#1661](https://github.com/JetBrains/resharper-unity/pull/1661))
- Rider: Add sample text for "Unity", "ShaderLab" and "Cg/HLSL" Colour Scheme options pages ([#1667](https://github.com/JetBrains/resharper-unity/pull/1667))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class RedundantAttributeOnTargetProblemAnalyzer : UnityElementProblemAnal
// UnityEngine
{KnownTypes.AddComponentMenu, AttributeTargets.Class},
{KnownTypes.ExecuteInEditMode, AttributeTargets.Class},
{KnownTypes.HideInInspector, AttributeTargets.Field},
{KnownTypes.HideInInspectorAttribute, AttributeTargets.Field},
// All but undocumented. Appears to have the same usage as ImageEffectOpaque
// The ImageEffect* attributes that are applied to methods are only useful
// when applied to OnRenderImage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ protected override void Analyze(IAttribute attribute, ElementProblemAnalyzerData
if (!(attribute.TypeReference?.Resolve().DeclaredElement is ITypeElement attributeTypeElement))
return;

if (!Equals(attributeTypeElement.GetClrName(), KnownTypes.HideInInspector))
if (!Equals(attributeTypeElement.GetClrName(), KnownTypes.HideInInspectorAttribute))
return;

var fields = attribute.GetFieldsByAttribute();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public AddHeaderAttributeAction(ICSharpContextActionDataProvider dataProvider)
{
}

protected override IClrTypeName AttributeTypeName => KnownTypes.Header;
protected override IClrTypeName AttributeTypeName => KnownTypes.HeaderAttribute;
protected override bool IsLayoutAttribute => true;

protected override AttributeValue[] GetAttributeValues(IPsiModule module, IFieldDeclaration fieldDeclaration)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public AddRangeAttributeAction(ICSharpContextActionDataProvider dataProvider)
{
}

protected override IClrTypeName AttributeTypeName => KnownTypes.Range;
protected override IClrTypeName AttributeTypeName => KnownTypes.RangeAttribute;
protected override bool IsLayoutAttribute => false;

// It can make sense to apply Range to all of the fields in a multiple field declaration, but it's unintuitive,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public AddSpaceAttributeAction(ICSharpContextActionDataProvider dataProvider)
{
}

protected override IClrTypeName AttributeTypeName => KnownTypes.Space;
protected override IClrTypeName AttributeTypeName => KnownTypes.SpaceAttribute;
protected override bool IsLayoutAttribute => true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public AddTooltipAttributeAction(ICSharpContextActionDataProvider dataProvider)
{
}

protected override IClrTypeName AttributeTypeName => KnownTypes.Tooltip;
protected override IClrTypeName AttributeTypeName => KnownTypes.TooltipAttribute;
protected override bool IsLayoutAttribute => false;

// It makes no sense to apply Tooltip to each field in a multiple field declaration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public bool IsAvailable(IUserDataHolder cache)
if (classLikeDeclaration == null)
return false;

var existingAttribute = classLikeDeclaration.GetAttribute(KnownTypes.CreateAssetMenu);
var existingAttribute = classLikeDeclaration.GetAttribute(KnownTypes.CreateAssetMenuAttribute);
return existingAttribute == null && UnityApi.IsDescendantOfScriptableObject(classLikeDeclaration.DeclaredElement);
}

Expand All @@ -81,7 +81,7 @@ protected override Action<ITextControl> ExecutePsiTransaction(ISolution solution
new Pair<string, AttributeValue>("order", new AttributeValue(new ConstantValue(0, myModule))),
};

var attribute = AttributeUtil.AddAttributeToSingleDeclaration(myClassLikeDeclaration, KnownTypes.CreateAssetMenu, EmptyArray<AttributeValue>.Instance,
var attribute = AttributeUtil.AddAttributeToSingleDeclaration(myClassLikeDeclaration, KnownTypes.CreateAssetMenuAttribute, EmptyArray<AttributeValue>.Instance,
values, myModule, myElementFactory);

return attribute.CreateHotspotSession();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public ToggleHideInInspectorAttributeAction(ICSharpContextActionDataProvider dat
{
}

protected override IClrTypeName AttributeTypeName => KnownTypes.HideInInspector;
protected override IClrTypeName AttributeTypeName => KnownTypes.HideInInspectorAttribute;
protected override bool IsRemoveActionAvailable => true;
protected override bool IsLayoutAttribute => false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.Linq;
using JetBrains.Annotations;
using JetBrains.Diagnostics;
using JetBrains.Metadata.Reader.API;
using JetBrains.Metadata.Reader.Impl;
using JetBrains.ProjectModel;
Expand All @@ -10,7 +12,6 @@
using JetBrains.ReSharper.Psi.Impl.Special;
using JetBrains.ReSharper.Psi.Modules;
using JetBrains.Util;
using JetBrains.Util.Dotnet.TargetFrameworkIds;

namespace JetBrains.ReSharper.Plugins.Unity.CSharp.Psi.CodeAnnotations
{
Expand All @@ -22,28 +23,27 @@ public class CustomCodeAnnotationProvider : ICustomCodeAnnotationProvider
private static readonly IClrTypeName ourMeansImplicitUseAttribute = new ClrTypeName(typeof(MeansImplicitUseAttribute).FullName);
private static readonly IClrTypeName ourPublicAPIAttribute = new ClrTypeName(typeof(PublicAPIAttribute).FullName);
private static readonly IClrTypeName ourUsedImplicitlyAttributeFullName = new ClrTypeName(typeof(UsedImplicitlyAttribute).FullName);
private static readonly IClrTypeName ourValueRangeAttributeFullName = new ClrTypeName(typeof(ValueRangeAttribute).FullName);
private static readonly IClrTypeName ourImplicitUseTargetFlags = new ClrTypeName(typeof(ImplicitUseTargetFlags).FullName);
// ReSharper restore AssignNullToNotNullAttribute

private readonly ExternalAnnotationsModuleFactory myExternalAnnotationsModuleFactory;
private readonly IPredefinedTypeCache myPredefinedTypeCache;
private readonly UnityApi myUnityApi;
private readonly IPsiModule myAnnotationsPsiModule;

public CustomCodeAnnotationProvider(ExternalAnnotationsModuleFactory externalAnnotationsModuleFactory, IPredefinedTypeCache predefinedTypeCache, UnityApi unityApi)
{
myPredefinedTypeCache = predefinedTypeCache;
myUnityApi = unityApi;
myAnnotationsPsiModule = externalAnnotationsModuleFactory
.GetPsiModule(TargetFrameworkId.Default);
myExternalAnnotationsModuleFactory = externalAnnotationsModuleFactory;
}

public CodeAnnotationNullableValue? GetNullableAttribute(IDeclaredElement element)
{
return null;
}

public CodeAnnotationNullableValue? GetContainerElementNullableAttribute(
IDeclaredElement element)
public CodeAnnotationNullableValue? GetContainerElementNullableAttribute(IDeclaredElement element)
{
return null;
}
Expand All @@ -52,6 +52,8 @@ public ICollection<IAttributeInstance> GetSpecialAttributeInstances(IClrDeclared
{
if (GetCoroutineMustUseReturnValueAttribute(element, out var collection)) return collection;
if (GetPublicAPIImplicitlyUsedAttribute(element, out collection)) return collection;
if (GetValueRangeFromRangeAttribute(element, out collection)) return collection;

return EmptyList<IAttributeInstance>.Instance;
}

Expand All @@ -68,15 +70,16 @@ private bool GetCoroutineMustUseReturnValueAttribute(IClrDeclaredElement element
var predefinedType = myPredefinedTypeCache.GetOrCreatePredefinedType(element.Module);
if (!Equals(returnType, predefinedType.IEnumerator)) return false;

// The ctorArguments lambda result is not cached, so let's allocate everything up front
var args = new[]
{
new AttributeValue(
new ConstantValue("Coroutine will not continue if return value is ignored",
predefinedType.String))
};
collection = new[]
{
new SpecialAttributeInstance(
ourMustUseReturnValueAttributeFullName, myAnnotationsPsiModule, () => new[]
{
new AttributeValue(
new ConstantValue("Coroutine will not continue if return value is ignored",
predefinedType.String)),
})
new SpecialAttributeInstance(ourMustUseReturnValueAttributeFullName, GetModule(element), () => args)
};
return true;
}
Expand All @@ -86,7 +89,7 @@ private bool GetCoroutineMustUseReturnValueAttribute(IClrDeclaredElement element
// So if applied to a class, only the class is marked as in use, while the members aren't. This
// provider will apply [UsedImplicitly(ImplicitUseTargetFlags.WithMembers)] to any type that
// has the old [PublicAPI] applied
private static bool GetPublicAPIImplicitlyUsedAttribute(IClrDeclaredElement element,
private bool GetPublicAPIImplicitlyUsedAttribute(IClrDeclaredElement element,
out ICollection<IAttributeInstance> collection)
{
collection = EmptyList<IAttributeInstance>.InstanceList;
Expand All @@ -102,18 +105,77 @@ private static bool GetPublicAPIImplicitlyUsedAttribute(IClrDeclaredElement elem
.FirstOrDefault();
if (meansImplicitUse?.Constructor == null || !meansImplicitUse.Constructor.IsDefault) continue;

// The ctorArguments lambda result is not cached, so let's allocate everything up front
var flagsType = TypeFactory.CreateTypeByCLRName(ourImplicitUseTargetFlags, element.Module);
var args = new[] {new AttributeValue(new ConstantValue(3, flagsType))};
collection = new[]
{
new SpecialAttributeInstance(
ourUsedImplicitlyAttributeFullName, element.Module, () => new[]
{new AttributeValue(new ConstantValue(3, flagsType))}
)
new SpecialAttributeInstance(ourUsedImplicitlyAttributeFullName, GetModule(element), () => args)
};
return true;
}

return false;
}

// Treat Unity's RangeAttribute as ReSharper's ValueRangeAttribute annotation
private bool GetValueRangeFromRangeAttribute(IClrDeclaredElement element,
out ICollection<IAttributeInstance> collection)
{
collection = EmptyList<IAttributeInstance>.InstanceList;

if (!(element is IField field) || !element.IsFromUnityProject()) return false;

// Integer value analysis only works on integers, but it will make use of annotations applied to values that
// are convertible to int, such as byte/sbyte and short/ushort. It doesn't currently use values applied to
// uint, or long/ulong, but it is planned, so we'll apply to all sizes of integer.
var predefinedType = myPredefinedTypeCache.GetOrCreatePredefinedType(element.Module);
if (!Equals(field.Type, predefinedType.Int) && !Equals(field.Type, predefinedType.Uint) &&
!Equals(field.Type, predefinedType.Long) && !Equals(field.Type, predefinedType.Ulong) &&
!Equals(field.Type, predefinedType.Short) && !Equals(field.Type, predefinedType.Ushort) &&
!Equals(field.Type, predefinedType.Byte) && !Equals(field.Type, predefinedType.Sbyte))
{
return false;
}

foreach (var attributeInstance in field.GetAttributeInstances(KnownTypes.RangeAttribute, false))
{
if (!myUnityApi.IsSerialisedField(field))
continue;

// Values are floats, but applied to an integer field. Convert to integer values
var unityFrom = attributeInstance.PositionParameter(0);
var unityTo = attributeInstance.PositionParameter(1);

if (!unityFrom.IsConstant || !unityFrom.ConstantValue.IsFloat() ||
!unityTo.IsConstant || !unityTo.ConstantValue.IsFloat())
{
continue;
}

// The check above means this is not null. We take the floor, because that's how Unity works.
// E.g. Unity's Inspector treats [Range(1.7f, 10.9f)] as between 1 and 10 inclusive
var f = Math.Floor((float) unityFrom.ConstantValue.Value.NotNull());
var t = Math.Floor((float) unityTo.ConstantValue.Value.NotNull());

// The ctorArguments lambda result is not cached, so let's allocate everything up front
var from = new AttributeValue(new ConstantValue(Convert.ToInt64(Math.Floor(f)), predefinedType.Long));
var to = new AttributeValue(new ConstantValue(Convert.ToInt64(Math.Floor(t)), predefinedType.Long));
var args = new[] {from, to};

collection = new[]
{
new SpecialAttributeInstance(ourValueRangeAttributeFullName, GetModule(element), () => args)
};
return true;
}

return false;
}

private IPsiModule GetModule(IClrDeclaredElement element)
{
return myExternalAnnotationsModuleFactory.GetPsiModule(element.Module.TargetFrameworkId) ?? element.Module;
}
}
}
Loading