Quick fix for access properties by id instead of string

[krasnotsvetov]

Access properties in Shader, Material, Animator by int is faster than using string overload.

The property IDs created from string hashes are deterministic over the course of a single run, so this quick fix creates a private static readonly field in class and reuse this field for overload.
Example:

public class SimpleTest
{
    public void Method(Material material)
    {
        material.SetFloat("test", 10.0f);
    }
}

is transformed to:

public class SimpleTest
{
    private static readonly int Test = Shader.PropertyToID("test");
    public void Method(Material material)
    {
        material.SetFloat(Test, 10.0f);
    }
}

[citizenmatt]

Looks good to me! Might be worth adding a couple more tests with property names beginning with underscore - I've seen this naming style used a lot in shaders. Make sure the field name get generated correctly, doesn't clash with existing name, etc.

[krasnotsvetov]

Add test with undescore name

[citizenmatt]

LGTM!

[TessenR]

What about string concatenation: M("part" + "secondPart") or constant strings: const string constStr = "myString"; M(constStr)?
This code won't work with any of this. I'm not sure whether not supporting them is a deliberate decision or an overlook, please add tests for these scenarios.

[TessenR]

Is this IDictionary<TKey, (TKey, TValue)> really necessary? Seems very confusing and any change to this code will require understanding why this type was introduced.
Looks like the only usage var (clrName, methodName) = ourTypes[containingType.GetClrName()]; could be easily rewritten without it:

var clrName = containingType.GetClrName();
var methodName = ourTypes[clrName];

[TessenR]

Probably worth introducing a variable for stringMethod.ShortName since it's mentioned at least 4 times in this method.

[TessenR]

Do these parameters allow nulls or not?
t.GetDeclaredElement() as IMethod is passed from the predicate filter, so nulls can be passed in,
however the first line uses them without null checks: if (!stringMethod.ReturnType.Equals(intMethod.ReturnType)) return false; so nulls will result in a NullReferenceException.

Annotating members with [NotNul] or [CanBeNull] could have caught possible NREs like this one, please consider using them.

[TessenR]

This allows methods with following signatures:
M(int firstId, int secondId)
M(string firstName, string secondName)
Probaby should only allow one int to string mismatch in parameters.

[TessenR]

Please move this code under the if (name == null) as it's not required for other code paths

[TessenR]

Looks like this change will break code with named arguments:

    public class InvalidLiteralForProperyNameTest
    {
        public void Test(Material material)
        {
            material.SetFloat(value: 10.0f, name: "f{caret}loat");
        }
    }

Please add tests for such cases

[TessenR]

I'd recommend using declared elements instead of strings. Relying on strings introduces additional bugs since created reference expression loses its context and might be accidently binded to another declared element that hides the desired one, e.g.:

public class TopLevelClass
{
  private static int Test  = Shader.PropertyToID("test");

  public class NestedClass
  {
    private static string Test = "notTest";

    public void Method(Material material)
    {
      material.SetFloat("te{caret}st", 10.0f);
    }
  }
}

I believe this test will use the nested class' Test property which is not correct.

[TessenR]

Please check all the invariants like having at least required number of arguments explicitly.
Current implemenation will work but it just barely misses bugs like a NullReferenceException on an invocation like this:
var s = nameof(Method);.

[TessenR]

material.SetFloat("f{caret}loat", 10.0f);`

is converted to

private static readonly int Property = Shader.PropertyToID("Property");
...
   material.SetFloat({caret}Property, 10.0f);

Are these code samples really equivalent?

[TessenR]

This code only checks declarations syntaxically located in the passed declaration.
I belive that my original example still won't reuse existing property, could you please add a test for it?

public partial class TestClass
{
  public void Method(Material material)
  {
    material.SetFloat("te{caret}st", 10.0f);
  }
}

public partial class TestClass
{
  private static readonly int Test = Shader.PropertyToID("test");
}

You can check all declarations of a type like this

foreach (var declarationPart in declaration.DeclaredElement.GetDeclarations())

[TessenR]

Looks like out var name is always overwritten before usage. Can we get rid of it?