DevTalk.net

ActiveMesa's software development blog. MathSharp, R2P, X2C and more!

ReSharper SDK Adventures Part 2 – Math.Pow Improvements

with 2 comments

In the first part of our SDK experiments, we implemented a way of identifying Math.Pow() calls with integer-based powers and wrote a quick-fix to correct the situation. Let us now try to improve the robustness of our code as well as increase its functionality.

Checking that caller is indeed System.Math

So far, we only checked that the function being called is Pow by getting the name of the reference:

bool functionIsCalledPow = false;
var e = element.InvokedExpression as IReferenceExpression;
if (e != null)
{
  if (e.Reference.GetName().Equals("Pow"))
    functionIsCalledPow = true;
}

This means that we can easily get a false positive with something like this:

double y = Foo.Pow(x, 2.0);

Now, to handle this situation properly, we’re going to scrap our previous check of the name and replace it with the following:

bool isOnMathPow = false;
var r = element.InvocationExpressionReference.Resolve();
var m = r.DeclaredElement as IMethod;
if (m != null)
{
  var parent = m.GetContainingType();
  if (parent != null)
  {
    isOnMathPow = parent.GetClrName().FullName.Equals("System.Math")
                  && m.ShortName.Equals("Pow");
  }
}

There’s quite a lot that’s happening in the above, so let’s go through it step-by-step:

  • The first thing you’ll notice is that we now use the InvocationExpressionReference and Resolve() it. This typically yields us a ‘resolve result’, but can also fail in case the method doesn’t resolve to anything.

  • To figure out if the resolution happened correctly, we simply take the result’s DeclaredElement and cast it to an IMethod, since Math.Pow() is a static method call.

  • If the result is OK, we also get the type that contains the method. If this is the right call, this should point us to the System.Math class.

  • To check that we do in fact have System.Math, we get the parent’s CLR name, and from that its FullName, which has the full namespace prefix.

  • To check we have the right method, we simply check its name as before.

The above manipulations, though complex, rid us from false positives like that Foo.Pow() call.

Custom function handling

Changing Math.Pow(x, 2.0) to x*x makes sense. So does changing Math.Pow(y, 3) to y*y*y. However, after this threshold, things get a bit ridiculous: you really don’t want z*z*z*z*z*z in your code, particularly since there’s no way to read from this that this is z^6. Wouldn’t it be nice if the user could specify their custom function that we ought to use for powers, say, greater than 3?

Let’s do this. First of all, we’ll define a settings class that will house the user’s preferences, specifically whether they want to have a custom power function, and what its name is:

[SettingsKey(typeof(Missing), "General Settings")]
public class GeneralSettings
{
  [SettingsEntry(false, "Use Custom Power Function")]
  public bool UseCustomPowerFunction { get; set; }
 
  [SettingsEntry("", "Custom Power Function Name")]
  public string CustomPowerFunctionName { get; set; }
}

We can now create a settings page (a user control that implements IOptionsPage) and bind the properties as follows:

settings.SetBinding(this.lifetime, (GeneralSettings gs) => gs.UseCustomPowerFunction,
  WinFormsProperty.Create(this.lifetime, chUseCustomFunction, x => x.Checked, true));
settings.SetBinding(this.lifetime, (GeneralSettings gs) => gs.CustomPowerFunctionName,
  WinFormsProperty.Create(this.lifetime, tbInliningFunctionName, x => x.Text, true));

Now, we need to change our inlining fix. This is tricky. First of all, let’s define a method that would actually get the two values given a context function:

public Pair<bool, string> GetCustomFunctionSettings(Func<Lifetime, DataContexts, IDataContext> ctx)
{
  var ss = Shell.Instance.GetComponent<ISettingsStore>();
  var bs = ss.BindToContextTransient(ContextRange.Smart(ctx));
  var s = bs.GetKey<GeneralSettings>(SettingsOptimization.DoMeSlowly);
  return new Pair<bool, string>(s.UseCustomPowerFunction, s.CustomPowerFunctionName);
}

The above simply gets a settings store, binds it to a transient context that’s based on the function we provide (more on that in a sec), reads the settings and returns them in a Pair<>.

The function above may be a bit tricky, but it’s required to create a context. Typically, though, you can get the function directly from whatever code element you’re operating on. In our case, the highlighting passed into a quick-fix has an Expression, so we can simply use that:

public IntPowerInliningFix(IntPowerHighlighting highlighting)
{
  this.highlighting = highlighting;
  customNameSettings = GetCustomFunctionSettings(highlighting.Expression.ToDataContext());
}

Now that we’ve got the settings, all that remains is to use them! The approach I’m going to take here is to use the custom function, if available, for powers greater than 3:

protected override Action<ITextControl> ExecutePsiTransaction(ISolution solution, IProgressIndicator progress)
{
  var expr = highlighting.Expression;
  var arg = expr.Arguments[0];
  var factory = CSharpElementFactory.GetInstance(expr.GetPsiModule());
  ICSharpExpression replacement;
  if (customNameSettings.First && highlighting.Power > 3)
  {
    var template = "$0($1, " + highlighting.Power + ")";
    replacement = factory.CreateExpression(template, 
      customNameSettings.Second, expr.Arguments[0]);
  }
  else
  {
    replacement = factory.CreateExpression(
      Enumerable.Range(0, highlighting.Power).Select(i => "$0").Join("*"), arg.Value);
  }
  ModificationUtil.ReplaceChild(expr, replacement);
      
  return null;
}

Now, a user can specify a replacement function Maths.IntPow and have their Math.Pow(x, 4.0) call refactored to Maths.IntPow(x, 4). Note that it’s 4, not 4.0 — we assume that the target function takes an int as the second parameter. Note also that in order to get 4 to appear, we cannot use the $ notation – instead, we prepare the template by concatenating strings, putting the integer there manually.

Code Cleanup

Changing the Math.Pow() invocations one at a time is too slow if you’ve got hundreds of such instances. The mechanism to perform changes en masse in ReSharper is called Code Cleanup and that’s the mechanism we’re going to use to perform changes on all int-bearing instances of Math.Pow() that are found, be it in a file, project or whole solution.

So let’s start with the basics: what we need is a code cleanup module, a class that implements the ICodeCleanupModule interface and is decorated with [CodeCleanupModule]. Unfortunately, ICodeCleanupModule has six methods that we must implement. Some of these are similar to IQuickFix methods but actually serve different purposes. There’s also the thorny issue of descriptors, so let’s start with those.

A descriptor is basically an option of code cleanup. For example, the option of whether to change declarations to var is encapsulated within a descriptor. We need at least one descriptor to define whether we want to do the Math.Pow() replacement at all, so here goes:

[DefaultValue(false)]
[DisplayName("Replace Math.Pow() integer calls")]
[Category(CSharpCategory)]
private class Descriptor : CodeCleanupBoolOptionDescriptor
{
  public Descriptor() : base("ReplaceMathPowIntegerCalls") {}
}

All this does is provide an option to turn the feature on or off in various code cleanup profiles. Now, this feature can be instantiated and returned from the code cleanup module:

private static readonly Descriptor descriptor = new Descriptor();
public ICollection<CodeCleanupOptionDescriptor> Descriptors
{
  get { return new[] {descriptor}; }
}

Now, moving on, let’s implement the IsAvailable() method. This method takes an IPsiSourceFile and we’re supposed to determine whether it can be used. Well, our only restriction right now is that this has to be a C# file, so…

public bool IsAvailable(IPsiSourceFile sourceFile)
{
  return sourceFile.GetPsiFile<CSharpLanguage>() != null;
}

One small thing to note is that the above causes the action to also fire in injected PSI, i.e., in C# that’s part of MVC or Razor views. If you want to avoid this, we could use GetNonInjectedPsiFile() instead.

Next, let’s go after SetDefaultSetting(), which determines the default value of this option depending on the code cleanup profile that’s being used. Let’s assume that, by default,

  • Our feature is on in the “Full Cleanup” profile.

  • Our feature is off in the “Reformat” profile.

Thus, the following implementation of SetDefaultSetting() communicates our preferences:

public void SetDefaultSetting(CodeCleanupProfile profile, CodeCleanup.DefaultProfileType profileType)
{
  switch (profileType)
  {
    case CodeCleanup.DefaultProfileType.FULL:
      profile.SetSetting(descriptor, true);
      break;
    default:
      profile.SetSetting(descriptor, false);
      break;
  }
}

Now, there’s also the IsAvailableeOnSelection property, which is pretty self-descriptive. You can return true or false here based on applicability; it doesn’t matter much for our experiments.

Finally, we come to the meat of the problem: the Process() method where changes actually happen. But, annoyingly enough, this method needs to reuse our analyzer and quick-fix code. We don’t really want to run it all again, do we? As a result, we perform the following refactorings:

  • In IntPowerProblemAnalyzer, we isolate the check on the right method into a static method called InvocationExpressionIsMathPowCall. This function now returns two values – a bool indicating whether this is the right function, and an int indicating the power that is being used.

  • In IntPowerInliningFix, we ensure the function to acquire settings is static; we then isolate the code that performs the change into a separate (again, static) method.

We can now attempt to perform the code cleanup itself. This is no mean feat, considering that previously, in our quick-fix, a lot of the plumbing was handled by the BulbItemImpl class that we inherited from. Let’s do a few checks, first. Let’s check that we’ve got the right file and that we are allowed to do the change:

var file = sourceFile.GetPsiFile<CSharpLanguage>();
if (file == null) 
  return;
 
if (!profile.GetSetting(descriptor)) 
  return;

If these checks pass, there’s some legitimacy to doing the change. We may as well get the user settings for the quick-fix right now, because doing them every time we meet an invocation expression is a bad idea:

var settings = IntPowerInliningFix.GetCustomFunctionSettings(sourceFile.ToDataContext());

Now, regrettably, we must manually set up a transaction using the PsiManager. We’re going to also be using shell locks, specifically a write lock that is required to perform changes in a file. The IShellLocks variable can be injected into the constructor of our module:

var settings = IntPowerInliningFix.GetCustomFunctionSettings(sourceFile.ToDataContext());
file.GetPsiServices().PsiManager.DoTransaction(() =>
{
  using (shellLocks.UsingWriteLock())
  {
    // scary stuff here
  }
}, "Code cleanup");

So, what exactly is happening inside the above? Well, we have to go through all IInvocationExpression’s in the file recursively and, for each one that happens to be a correct Math.Pow() call, perform the replacement. This is where our refactorings come into play, because without them we’d be duplicating code or creating meaningless (and quite possibly broken) instances of our analyzer or quick-fix.

Here’s what the ‘scary stuff’ looks like:

var itemsToChange = new List<Pair<IInvocationExpression,int>>();
file.ProcessChildren<IInvocationExpression>(e =>
{
  int power;
  if (IntPowerProblemAnalyzer.InvocationExpressionIsMathPowCall(e, out power))
    itemsToChange.Add(new Pair<IInvocationExpression, int>(e, power));
});
foreach (var e in itemsToChange)
  IntPowerInliningFix.PerformChange(e.First, settings, e.Second);

Notice how we have to cache the items before they are processed — this is necessary because modifying these items as they are being iterated is not safe, just as it’s not safe to modify the loop iterator within the loop. Thus, we use a List<> to cache the items and their respective powers, and then reprocess them wholesale once the iteration has been completed.

Conclusion

In this post I’ve demonstrated how to dig into the invocation’s type name (no easy feat), how to define settings and use them. I’ve also demonstrated how to write a code cleanup module to perform large-scale changes on the code base. You can find the source code here.

Written by Dmitri Nesteruk

May 10th, 2012 at 12:07 pm

Posted in ReSharper

  • http://team23.ru derigel


    bool isOnMathPow = false;
    var r = element.InvocationExpressionReference.Resolve();
    var m = r.DeclaredElement as IMethod;
    if (m != null)
    {
      isOnMathPow = m.XMLDocId == "M:System.Math.Pow(System.Double,System.Double)";
    }

    • http://devtalk.net Dmitri

      Yep, I imagine this would work too, though it’s a slightly less comprehensible alternative. There’s a hundred ways to skin the cat, as they say :)