search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.37k stars 4.75k forks source link

Then, what's the status of Code Contracts usage in .NET Core? #23869

Closed LYP951018 closed 4 years ago

LYP951018 commented 7 years ago

Related: https://github.com/Microsoft/CodeContracts/issues/231 If the contracts feature is not usable, why included in corefx?

karelz commented 7 years ago

@LYP951018 what is included in CoreFX? Can you share a link/reference?

tarekgh commented 7 years ago

@karelz this is https://github.com/dotnet/corefx/tree/master/src/System.Diagnostics.Contracts

and here is an example of the usage

https://github.com/dotnet/corefx/blob/6bf1e7c3032d3c24d5a41bbd880549ef4a8de2da/src/System.Net.Http/src/System/Net/Http/FormUrlEncodedContent.cs#L28

karelz commented 7 years ago

Seems to be usable. Please provide additional details / repro if you think it is not completely true.

yaakov-h commented 7 years ago

Code Contracts in .NET Framework provides API surface only, but requires the use of a rewriter.

For example, see http://referencesource.microsoft.com/#mscorlib/system/diagnostics/contracts/contracts.cs,388

The current tools (including the rewriter) provided by the Microsoft Research Code Contracts project does not work with .NET Core and the new project system. The project is abandoned and all but officially discontinued.

In this context, the OP's question, which I'm curious to know as well, is twofold:

  1. Why is the System.Diagnostics.Contracts namespace available if the APIs are unusable?
  2. Why does CoreFX code internally call Contract methods such as the usage shown above? The only reason this makes sense is if either:
    1. The Code Contracts static analysis is performed on CoreFX.
    2. The contracts are used to automatically generate CoreFX Contract Reference Assemblies

As far as I know, neither of the last two points are true.

tarekgh commented 7 years ago

CC @weshaggard as I recall this issue was triggered before.

LYP951018 commented 7 years ago

@karelz No, it's not usable. Without CONTRACTS_FULL, the Contracts code compiles to nothing, nothing! With CONTRACTS_FULL being defined, programs using Contracts could not run because lacking the rewrite tool: CCRewrite.

stephentoub commented 7 years ago

Why is the System.Diagnostics.Contracts namespace available if the APIs are unusable?

So that a) code that's written with the APIs can still compile, b) use of the APIs that don't require CONTRACT_FULL (like Contract.Assert) still work, and c) the APIs can be used with the tooling in the future if/when it becomes available.

Why does CoreFX code internally call Contract methods such as the usage shown above?

Because it simply wasn't removed when it was brought over from the original .NET Framework code. New usage hasn't been added, and lots of usage has been replaced (e.g. replacing Contract.Assert with Debug.Assert), but we've yet to do a full sweep to clean out the old usage. You can see @danmosemsft recently did a sweep in the coreclr repo (https://github.com/dotnet/coreclr/pull/14136), and I imagine he's eyeing corefx next. Such work is tracked by https://github.com/dotnet/corefx/issues/15443.

LYP951018 commented 7 years ago

@stephentoub

a) code that's written with the APIs can still compile

"Still compile" is really bad. Imagine someone didn't know CONTRACTS_FULL and thought that Code Contract just works, then he used Contract.Requires Contract.Ensures everywhere and compiled and ran his program. Of cource the program ran smoothly and none of his "Code Contracts" was violated. Until one day, he found Code Contracts only works with CONTRACTS_FULL, then he defined that macro, compiled and ran again. Then he heard that Code Contracts only works after rewriting by CCRewrite. Finally, he found that CCRewrite is unavaliable on VS2017/.NET Core. He is disappointed. That foolish boy is me. It should be documented that Code Contracts' APIs are included in CoreFx but almost none of them works ("Still compile" isn't "work".) and some compiler warnings should be given.

the APIs can be used with the tooling in the future if/when it becomes available.

When? It seems that the Code Contracts repo has been abandoned, and the forum is almost dead. Will it become avaliable? And, I found little document about the implementation/source code of Code Contracts. That means porting by community is... hard, even impossible.

yaakov-h commented 7 years ago

It's probably too late to remove them now that it's in .NET Standard, but perhaps it could be obsoleted to prevent compatibility assumptions like the OP made?

Code Contracts is also an extraordinarily complex academic project, so there's no hope for "the community" picking up the pieces and giving the project new life.

It's almost certain that no third party tooling will ever use Assume, Requires, Ensures, Ensures<> or Invariant. Most of Contract's static analysis is better served by Roslyn Analysers anyway, or C#8's Nullable Reference Types.

stephentoub commented 7 years ago

"Still compile" is really bad. Imagine someone didn't know CONTRACTS_FULL

That's a valid concern with the design of the Contracts library in general, but that has nothing to do with corefx; it's always been like that.

karelz commented 7 years ago

If the APIs are useless today, we should obsolete them in https://github.com/dotnet/platform-compat ... we can even obsolete them on specific platforms if needed.

AceHack commented 6 years ago

I'm really going to miss code contracts, I would like to see them in .NET core. Very helpful tool.

jzabroski commented 6 years ago

@yaakov-h

It's probably too late to remove them now that it's in .NET Standard, but perhaps it could be obsoleted to prevent compatibility assumptions like the OP made?

Code Contracts is also an extraordinarily complex academic project, so there's no hope for "the community" picking up the pieces and giving the project new life.

It's not an "extraordinarily complex academic project". The reason it never got mainstream support funding, in my understanding, is that it's not able to handle threading. Likewise, Pex and Moles couldn't handle concurrency checks either.

It's almost certain that no third party tooling will ever use Assume, Requires, Ensures, Ensures<> or Invariant. Most of Contract's static analysis is better served by Roslyn Analysers anyway, or C#8's Nullable Reference Types.

Could you elaborate?

yaakov-h commented 6 years ago

It's not an "extraordinarily complex academic project".

I'm gonna have to ask for proof on that. From what I can tell, as a complete outsider:

I've tried to make fixes to the project, but I've only been able to scratch the surface. It's horribly complicated - perhaps by necessity, given what it's trying to achieve - but the point still stands.

Could you elaborate?

Contracts had enough of a push within Microsoft to get the .NET Framework team to add System.Diagnostics.Contracts to the BCL in .NET Framework 4.0, instead of being a separate assembly distributed by Code Contracts. These were APIs that in theory anybody could use, but only ever had one implementer.

The Contract team appear to have done some premature abstraction here - creating a generic solution when there is only one implementation is usually a bad idea and will require reworking the abstraction. (You can tell that the APIs were targeted at Code Contracts specifically because they're in System.Diagnostics.Contracts and require the CONTRACTS_FULL symbol to be defined.)

Those APIs, Assume, Requires and friends, do nothing. Requires<TException> does nothing but throws. In order to use them, you need something that will rewrite those calls, either as part of the compilation pipeline in Roslyn on in IL.

Rewriting IL is almost certainly setting you up for failure. You need to keep up with not only the capabilities of .NET, but the different manners in which various compilers emit different IL to do the same thing. Code Contracts has not, and so there is code today that is perfectly valid C# 5.0 syntax that the rewriter and/or static checker totally crap themselves on. If you were going to start a new project today that does this, you're in for one big long-term headache.

The only other option is to hook into the compiler itself. Roslyn does not have any support for code generators or code rewriters (this is something often discussed on dotnet/csharplang), but it does have support for running third-party code to produce errors and warnings.

This suits the static checker much better - Roslyn has syntax, semantic model, and flow analysis. C# 8.0 will (hopefully) introduce nullable reference types, which will make heavy use of flow analysis.

So, if all you're getting out of Code Contracts is Contract.Requires(foo != null), C# 8.0 should obviate your need for Code Contracts entirely.

For anything else, if you can express it in attributes and annotate your code, a Roslyn Analyser can do that too for other things (e.g. [NotNullOrEmpty] string foo. This is how other similar analysis tools such as Jetbrains' works.

Ergo, it's almost certain that no third party tooling will ever use Assume, Requires, Ensures, Ensures<> or Invariant. Most of Contract's static analysis is better served by Roslyn Analysers anyway, or C#8's Nullable Reference Types.

weitzhandler commented 4 years ago

Related: https://github.com/dotnet/csharplang/issues/105.

Anyway, what's the current alternative to CC as of Q2 2020?