dotnet / roslyn-analyzers

MIT License
1.58k stars 463 forks source link

Codefixers to move banned API usage to recommended APIs #5696

Open twsouthwick opened 2 years ago

twsouthwick commented 2 years ago

Describe the problem you are trying to solve

I have been working on identifying strategies of how to move customers away from types available in .NET Framework that are not available as they move to .NET Core. The driving scenario of this is HttpContext and members off of that (although the type itself exists in ASP.NET Core, it is not the same by any means). However, it is a general purpose need of not only knowing you shouldn't use an API but what should be used instead, especially in large codebases where automation of the codefixers can save a lot of time. I'd like to not only highlight the error as the BannedApi analyzers do, but provide code fixers that will help consumers of the library to move forward.

The question I have is if this would fit in with the goals of the banned API analyzer, if it would be a good general purpose analyzer housed here, of if I should put it somewhere else.

Describe suggestions on how to achieve the rule

I have developed a set of analyzers/codefixers that identify types and members that the developer has marked as deprecated (the terminology I was using until I found the Banned API analyzers and realized there may be value in augmenting the functionality there) and can also define what should be done. Some of the behavior of this includes:

Example

Below is an example of what I currently have. There's also support for defining static methods that should not be used and an alternate one that should be that I'm not showing (this is, for example, useful for things like HttpContext.Current and ClaimsPrincipal.Current). The following example is a basic flow of applying a few analyzer/codefixers that can transition users from banned APIs to recommended ones:

  1. A code base has the following code:
using System.Web;

public void EntryPoint()
{
  SomeMethod(HttpContext.Current);
}

public void SomeMethod(HttpContext context)
{
  var name = context.Request.Headers["some-header"];
}
  1. The analyzer identifies that the developer is using a known "banned api", i.e. HttpContext and provides a fix to add the following attributes (with the declaration of the type as well).
[assembly: AttributeDescriptor(typeof(System.Web.HttpContext))]
[assembly: AttributeDescriptor(typeof(System.Web.HttpRequest))]
  1. Next, another analyzer provides a fix to generate an abstraction:
[assembly: AttributeDescriptor(typeof(System.Web.HttpContext), typeof(IHttpContext))]
[assembly: AttributeDescriptor(typeof(System.Web.HttpRequest), typeof(IHttpRequest))]

public interface IHttpContext
{
}

public interface IHttpRequest
{
}
  1. The analyzer detects that the user is calling the member HttpContext.Request.Headers and provides a fix that will add that to the abstraction:
public interface IHttpContext
{
  IHttpRequest Request { get; }
}

public interface IRequest
{ 
  NameValueCollection Headers { get; }
}

Note: this translates the HttpRequest to the defined abstraction of IHttpRequest

  1. An analyzer detects that the method is taking in an HttpContext and offers to change it to IHttpContext:
using System.Web;

public void EntryPoint()
{
  SomeMethod(HttpContext.Current);
}

public void SomeMethod(IHttpContext context)
{
  var name = context.Request.Headers["some-header"];
}
  1. The developer can add a factory method and add an attribute to teach the analyzer about its existence:
[assembly: Microsoft.CodeAnalysis.Refactoring.AdapterFactoryDescriptor(typeof(HttpContextFactory), nameof(HttpContextFactory.Create))]
  1. Now, an analyzer detects a method that expects an IHttpContext but is passing an HttpContext and offers to fix it:
using System.Web;

public void EntryPoint()
{
  SomeMethod(HttpContextFactory.Create(HttpContext.Current));
}

public void SomeMethod(IHttpContext context)
{
  var name = context.Request.Headers["some-header"];
}
twsouthwick commented 2 years ago

@jmarolf I'm not certain what the internal setup PR has to do with this issue...

jmarolf commented 2 years ago

sorry, pasted and closed into the wrong issue