sergeyteplyakov / errorprone.net Goto Github PK
View Code? Open in Web Editor NEWSet of roslyn-based analyzers for catching common C# errors (inspired by Google's error-prone)
License: MIT License
Set of roslyn-based analyzers for catching common C# errors (inspired by Google's error-prone)
License: MIT License
Consider following case:
public async Task<object> Foo() {
return await Task.FromResult("foo");
}
Removing async and await like this:
public Task<object> Foo() {
return Task.FromResult("foo");
}
Will lead to an invalid code!
The compiler knows about the nullable types are immutable and causes no hidden copies for them.
Tasks in many cases are pure. I.e. if the await expression returns something this should be a warning.
Some examples could be found here: https://github.com/ljw1004/blog/blob/master/Analyzers/MissingAwaitAnalyzer/MissingAwaitAnalyzer/readme.md
Do to suggest: pass by in if the argument is reassigned. argument is used in an anonymous method.
Formatting issue with the fixer that adds 'in'. Need to carry trivia. Done
Do not suggest using in for task-based methods, not only for async methods. (it could be task-like thing, not only task!) Done.
Warn if 'in'-modifier is used for Task-based method. Done
Analyze indexers for 'use 'in' modifier. Done
Remove primitives from 'non-readonly struct 'int32' used as in-parameter.
Do not convert argument if it is used in out context.
Do not warn about 'non readonly struct is used as in' if the method overrides. Done
Consider the following case:
public void Run(Action<int> a) {
a();
}
try
{
Run(async n => Parse(n));
}
catch(Exception) {}
This method will crash the app. This issue is very hard to spot.
Warn if the method is not presented or could not be found, or return value is not string etc.
With a fixer to use a bit-wise comparison.
Warn if the result is not observed.
Otherwise the tool will notify that catch block swallows an exception even that it doesn't.
Sample: https://twitter.com/ViIvanov/status/753602621651509248
Automate the build using appveyor.
Automate publishing new prerelease version when the build is successful in master.
new Excpetion
without throw
(DONE)if (arg == null) {
new ArgumentException("arg");
}
try {
}
catch(Exception ex) {
throw ex; // should be throw;
}
if
(DONE by compiler)if (foo); {
Console.WriteLine("foo is true");
}
if (a == a) {
}
// The same for equals, object.Equals
var x = 42;
x = x;
Enumerable.Range(1, 10);
This check can rely on simple knowledge what methods are pure (string, enumerable, delegate, immutable collections, domino collections).
Emit a warning for calls ToString on collections. Fix could introduce string.Join(", ", collection).
Emit a warning for calling Equals on collections... Is it possible?
This one should be easy. Fix could be to switch to interpolated string!
Add custom attribute (use ducktyping and R# attributes) to warn on those method invocations as well!!
This could be very fun rule to implement!
Idea: http://errorprone.info/bugpattern/NoAllocation
Use Count() instead of Count? Count() != 0 instead of Any()?
Problem: how to check that the method is not-pure? Void-returned?
This could be an attribute on the assembly and if its defined, than my exception rule will work and break the build!
Example:
public async Task<T> Get<T>()
{
return await InnerGet<T>();
}
Explanation, msdn
If the method that calls the GetCallingAssembly method is expanded inline by the just-in-time (JIT) compiler, or if its caller is expanded inline, the assembly that is returned by GetCallingAssembly may differ unexpectedly. For example, consider the following methods and assemblies:
- Method M1 in assembly A1 calls GetCallingAssembly.
- Method M2 in assembly A2 calls M1.
- Method M3 in assembly A3 calls M2.
When M1 is not inlined, GetCallingAssembly returns A2. When M1 is inlined, GetCallingAssembly returns A3. Similarly, when M2 is not inlined, GetCallingAssembly returns A2. When M2 is inlined, GetCallingAssembly returns A3.
This effect also occurs when M1 executes as a tail call from M2, or when M2 executes as a tail call from M3. You can prevent the JIT compiler from inlining the method that calls GetCallingAssembly, by applying the MethodImplAttribute attribute with the MethodImplOptions.NoInlining flag, but there is no similar mechanism for preventing tail calls.
Actually there's undocumented trick in the BCL to prevent the behavior described above. Hovewer, I'd prefer to have a warning for code like this.
As far as I can remember the same error is possible when calling the MethodBase.GetCurrentMethod()
, calling the StackTrace()
constructor with skipFrames
arg set, calling the GetExecutingAssembly()
and so on.
As a sidenote: the project should be definitely added into awesome-analysers list.
Technically it would be good to proof non-nullability of the result, but even warning on return null
for TAP would be good.
I think it should be okay to copy small (64-bits) struct, shoudn't it?
If the struct is stored in readonly field, result of the invocation should be on the left hand side! Otherwise it will not have any effects.
Void return method should be an exception, because it is hard to check purity of the method.
Catch errors for potentially wrong equals/gethashcode.
First: mismatch between gethashcode and equals. Second, if something looks like a part of the state, just warn if that part is not used in the equality. Fixer can fix this stuff (maybe).
This is controversial check and maybe some other tool should check this.
But I found very useful for all boolean arguments have names on the invocation side.
It could sounds very simple and unlikely to happen, but I just got the situation when I've assigned property to itself!
And as far as I know, there was even a bug in Roslyn for self-assignment for arguments.
In many cases I need init-once readonlyness, when instance can be instantiated using object-initialization syntax with inability to change properties after construction.
Analyzer methods with Pure
attribute: warn if method changes the state.
One caveat here: technically speaking, pure method can change state if this change is invicible to the client. This is similar to physical and logical const-ness in C++. Logically, method could be const, even when it is changing the state.
THe same is true for pure methods (at least based on current semantic of the PureAttribute
that is described in the official documentation). I.e. method that implements cache-aside pattern could be considered as a pure method.
The idea is to add design time analyzer that will duplicate C# warning for unused fields in design time.
Roslyn team members mentioned that design time implementation is too time consuming and this could be correct, but anyway I would like to give it a try.
Current impementation of the CS0169 is following:
InternalVisibleToAttribute
defiend for the assemblyConsider following code:
private static async Task<string> ReadFileContentAsync(Path spec)
{
using (var reader = File.OpenText(spec.AbsolutePath))
{
return await reader.ReadToEndAsync();
}
}
In this case, removing await and async will change the behavior of the code!
So the rule should respect this.
Potentially the following code could be unsafe:
using System;
public class C {
public void M() {
int n = 42;
Foo(n);
Action a = () => {Console.WriteLine(n); n++;};
a();
}
public void Foo(in int x) {}
}
If the Foo
method is slow and a
delegate is called from a different thread, the argument observed in Foo
may be different at different points in time.
Even though it was always possible to do the same by passing an argument by ref.
Here is an example, when the analyzer still could show the warning:
struct Immutable {
private int _x;
public Immutable(int x) => _x = x;
}
But in this case, the field should be made readonly as well.
This is the case:
[DebuggerDisplay("{Foo,nq}")
class FooBar {
private string Foo() {return string.Empty;}
}
async void
Task.Result
/Task.Wait
prefer Task.GetAwaiter().GetResult
because later will unwrap exception. Question: how to catch correct cases?Enumerators for BCL collections (and any struct-based iterators)
SpinLock
Analyze the BCL to find other instances.
Calling string.Clone() is never ever useful. Nevertheless DevDiv has 19 references to it :)
Code is in attachment. I can't find any hidden copies in IL.
Suggest to convert it to pass by 'in'.
The Roslyn analyzer is primarily uses during development, so the default package reference should be privateAssets="all", e.g.
<PackageReference Include="ErrorProne.NET.Structs" Version="0.1.0" PrivateAssets="all" />
This will remove the package dependency from any generated nuget package.
try
{
//
}
catch (Exception ex) when (ex is FormatException || ex is OverflowException)
{
// common code
}
Real-world typo:
var newGuid = new Guid(); // Guid.NewGuid(); assumed
Add an attribute NoHeapAllocation
and AllowHeapAllocation
.
User can put an attribute on assembly, class or member level. If the attribute is present than the member should follow some heap allocation rule. If the rule is saying that allocations are prohibited, analyzer will warn an any heap allocations.
See controlflow stuff as an example here: https://github.com/controlflow/resharper-heapview
Especially for properties to itself!
In this case, the warning should be different and it should be covered by the memory analyzer.
For instance, in new ArgumentException("argName")
to switch to new ArgumentException(nameof(arg))
.
This method lead to additional allocation without no clear benefits.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.