Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
LYP951018 opened this issue Oct 17, 2017 · 15 comments
Closed

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

LYP951018 opened this issue Oct 17, 2017 · 15 comments
Labels
area-System.Diagnostics question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@LYP951018
Copy link
Contributor

Related: microsoft/CodeContracts#231
If the contracts feature is not usable, why included in corefx?

@karelz
Copy link
Member

karelz commented Oct 17, 2017

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

@karelz
Copy link
Member

karelz commented Oct 18, 2017

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

@karelz karelz closed this as completed Oct 18, 2017
@yaakov-h
Copy link
Member

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
Copy link
Member

tarekgh commented Oct 19, 2017

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

@LYP951018
Copy link
Contributor Author

LYP951018 commented Oct 19, 2017

@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
Copy link
Member

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 (dotnet/coreclr#14136), and I imagine he's eyeing corefx next. Such work is tracked by https://github.com/dotnet/corefx/issues/15443.

@LYP951018
Copy link
Contributor Author

LYP951018 commented Oct 19, 2017

@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
Copy link
Member

yaakov-h commented Oct 19, 2017

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
Copy link
Member

"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
Copy link
Member

karelz commented Oct 19, 2017

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
Copy link

AceHack commented Apr 17, 2018

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

@jzabroski
Copy link
Contributor

@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
Copy link
Member

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:

  • It was written and published by Microsoft Research, not Microsoft DevDiv.
  • The code reads similar to most cryptographic code, i.e. it appears to be written by people whose area of expertise is not in writing code, but they need to write code in order to solve their problem.
  • It wasn't written by run-of-the-mill software developers, but by PhDs and university interns.
  • The entire project was "open-sourced" (git repo created, pushed to GitHub, Microsoft tried to push it onto the .NET Foundation) in the weeks surrounding the project lead and co-creator, Francisco Logozzo, leaving Microsoft Research for a role at Facebook. It seems like it was fairly dead and that this was probably his way of making sure that it didn't just get thrown into the trash.

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
Copy link
Contributor

Related: dotnet/csharplang#105.

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

@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Diagnostics question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

9 participants