Forbid calls to arbitrary functions/classes in external code

Question

I've experienced cases where it would be valuable to restrict access to the API of external libraries and frameworks to prevent negative consequences in the system.

For example, in a SharePoint application it might seem natural to call spList.Items.GetItemById to get a list item, even maybe in a loop, without realizing that this can lead to huge performance problems.

It could also be that we need to forbid the usage of SmtpClient in order to force everyone to use our own class to send email, to ensure that we can properly proxy and mock all emailing when in the testing environment.

Are there any reliable and reasonably straightforward ways to achieve these constraints on external code, except from certain specific places in our own code? It's not necessary to absolutely under every circumstance prevent access to these methods/classes, for example by reflection or just some kind of disabling, it should rather be a strict warning that they should not be used. Preferably forcing the programmer to actively take measures to override these constraints if possible/needed.

Answer 1

Are there any reliable and reasonably straightforward ways to achieve these constraints on external code, except from certain specific places in our own code?

As the question is specifically about C#, there is a compiler-based solution that can be used here to enforce such rules: Roslyn Analyzers. You could write your own analyzer that reports accessing certain APIs as a compilation error or warning.

An example set of analyzers, that provide lots of example code on writing your own, are the StyleCop Analyzers, which are a replacement for the old StyleCop feature for C#.

Having said that, such automated checks can always be worked around by folk determined to "break the rules". Therefore this approach is not a substitute for code reviews as discussed in Karl Bielefeldt's answer. It can assist with such reviews, but should not replace them.

Answer 2

You can do time-consuming things like writing a wrapper around the external API that leaves out your undesired operations, but nothing beats training and code reviews, because whatever standards or technical measures you put in place, people will find creative ways to get around them.

For example, we have several services written in Scala, and one of the things we ask at code review time is for immutability, but we often communicate that as getting rid of the vars. Someone the other day used a val x: ListBuffer[Boolean] to hold a single mutable variable as the only item in the list. You can't assign another ListBuffer to x, but you can replace the items of the list in place as much as you want. Just as bad as using a var, but sneakier.

In other words, you have to check if people are going around your technical solutions. If those technical solutions are costly and add complexity, you may as well just check that they are coding it correctly.

Answer 3

By now, there is a specific analyzer available specifically for such a task: https://github.com/DotNetAnalyzers/BannedApiAnalyzer

You have to define a ruleset in BannedSymbols.txt. In your case, BannedSymbols.txt would contain something like

M:Microsoft.SharePoint.SPListItemCollection.GetItemById(System.Int32);Potential performance problems
T:System.Net.Mail.SmtpClient;Use OurCompany.FancyClient instead 

During compilation, you would then get warning messages

Answer 4

To elaborate on the "training and code review" suggestion raised in another answer: since the code you wish to forbid is legal code, you can't count on the compiler preventing it, and you'll have to rely on a later process, the review.

This can (and should) include both manual and automatic review steps:

• Prepare a checklist of known issues and go over them in your manual code reviews, one by one. Have a recurring meeting to review and update the checklist. Whenever a nasty bug is caught and analyzed, add it to the checklist.

• Add check in rules to look for known patterns. This can be complicated to write, but for a large project, over time, might be useful. TFS allows you to write rules in C#, and other build systems have their own hooks. Consider using gated builds to reject check-ins that match the pattern. Yes, it slows development, but after a certain project size and complexity, slowing down devs can be a good thing.

Answer 5

Karl's answer is 100% correct. There is no way to guarantee conformance. However, in addition to training and code reviews, consider the use of static analysis tools to ensure compliance. (Note: I said "in addition to", as one can bypass those as well in exactly the same way that Karl stated).

The advantage to using static analysis tools is one of removing tedious human code analysis looking for instances of "multiple use of IEnumerable" or whatever performance issue of the week you're looking at (or, at least, that I always feel I'm looking at). This will allow the code reviews and training to focus on more "interesting" issues.

For C#, specifically, I've included some suggestions below. Plug these into your build environment and you're good to go. But, generally, no matter what language you're using, there's a static analysis tool out there somewhere.

Copy/paste straight from the Wikipedia page, use the wiki page for the most recent information and links: https://en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis#.NET

• .NET Compiler Platform (Codename Roslyn) – Open-source compiler framework for C# and Visual Basic .NET developed by Microsoft .NET. Provides an API for analyzing and manipulating syntax.
• CodeIt.Right – Combines static code analysis and automatic refactoring to best practices which allows automatic correction of code errors and violations; supports C# and VB.NET.
• CodeRush – A plugin for Visual Studio which alerts users to violations of best practices.
• FxCop – Free static analysis for Microsoft .NET programs that compiles to CIL. Standalone and integrated in some Microsoft Visual Studio editions; by Microsoft.
• NDepend – Simplifies managing a complex .NET code base by analyzing and visualizing code dependencies, by defining design rules, by doing impact analysis, and by comparing different versions of the code. Integrates into Visual Studio.
• Parasoft dotTEST – A static analysis, unit testing, and code review plugin for Visual Studio; works with languages for Microsoft .NET Framework and .NET Compact Framework, including C#, VB.NET, ASP.NET and Managed C++.
• Sonargraph – Supports C#, Java and C/C++ with a focus on dependency analysis, automated architecture check, metrics and the ability to add custom metrics and code-checkers.
• StyleCop – Analyzes C# source code to enforce a set of style and consistency rules. It can be run from inside of Microsoft Visual Studio or integrated into an MSBuild project.

Answer 6

Maybe the compiler can help you to catch unwanted calls.

Rename classes/methods of code in your own lib that should not be used by external lib clients. Alternativly make classes/methods internal and add internals visible to to those classes that are allowed to use them.

External lib users will get a compile error method/class not found.

Forbidden Classes/methods from public libraries: create the same namespace/class/method in your lib

External lib users will get a compile error because of duplicate class found

[update]

It's not necessary to absolutely under every circumstance prevent access to these methods/classes, for example by reflection or just some kind of disabling, ....

forcing the programmer (... client of the lib...) to actively take measures to override these constraints if possible/needed.