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

replace JetBrains Annotations with CodeContracts. #6533

Closed
mwwhited opened this issue Sep 15, 2016 · 12 comments
Closed

replace JetBrains Annotations with CodeContracts. #6533

mwwhited opened this issue Sep 15, 2016 · 12 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@mwwhited
Copy link

Microsoft put Code Contracts in the BCL several years ago. They should be used instead of JetBrains.Annotations.

@roji
Copy link
Member

roji commented Sep 15, 2016

While at Microsoft, Code Contracts never made it out of research status. Moreover, the project is more or less dead now, with no support for .NET Core or regular updates.

Also, for its uses in Entity Framework Core (i.e. null checking), it is far from providing what Resharper can do with Jetbrains.Annotations - the contract language is cumbersome (Contract.Requires() as opposed to simple [NotNull] and [CanBeNull] attributes), and its static analysis is an extremely slow offline process, whereas Resharper works in realtime as you're developing.

In addition, non-nullable references are a planned feature in the future of C#, at which time 90% of the use of Jetbrains.Annotations (and Code Contracts as well) will be obviated. All this make Code Contracts a pretty poor choice at this point in time.

@mwwhited
Copy link
Author

Code contracts are in the BCL and the tooling is updated regularly. It would be nice f the static checker was in the box... but you have to install the jetbrains tools too. CodeContracts can also do a heck if a lot more than just NotNull.

@mwwhited
Copy link
Author

@roji
Copy link
Member

roji commented Sep 15, 2016

@mwwhited the Code Contract APIs (e.g. Contract.Requires) are in the .NET Core BCL, so that programs which use them can compile. However, the APIs themselves do nothing - you need the tooling for anything to happen, either the rewriter (for runtime checks) or the static analyzer.

I wouldn't say that the tooling is updated regularly - the last commit is from 3 months ago, and apart from merging some PRs nothing has really happened for a very long while. There's no support for .NET Core, and AFAIK there's nobody at Microsoft still officially working on this, you may want to check out microsoft/CodeContracts#409.

Note that CC's static analysis never got anywhere close to being fast enough to be part of the realtime development process. This is where R# really shines - you can see issues immediately as you program (like the more basic compilation errors/warnings), rather than some background build process which reports issues much later.

Finally, it's true that CC can do much more than null checks, although it seems that null checks are the vast majority of what people use CC for. Regardless, you'd be asking the EFCore team not just to switch to CC, but rather to spend time to code up contracts all across the codebase. In other words, it's not just a technology switch, but a lot of additional investment in a tool that's not even being actively maintained anymore.

Don't get me wrong, I was a big fan of CC and used it extensively in projects, it's just that the project seems pretty much dead at this point.

@bricelam
Copy link
Contributor

bricelam commented Sep 15, 2016

We used to use Code Contracts in EF6, but there were two main problems we had with it:

  1. It rewrites the invocation site to perform the check before calling the method. This resulted in much larger assemblies than necessary.
  2. We started finding a lot of issues in corner cases. For example when the contract is on an internal interface that gets implemented on a public type. (Or something like that--I don't remember the exact details.) These issues weren't addressed by the team as fast as we would have liked resulting in a lot of compensating code in our codebase.

@mwwhited
Copy link
Author

This isn't an CC forum but that is just because of how you configured your contracts. You can turn off runtime checking and it will actually decrease your code size and increase performance. It's also really easy to put contracts on your interfaces so you don't have to reimplment them everywhere.

@ajcvickers
Copy link
Member

@mwwhited To add to what @bricelam said, here are the notes from the meeting where we decided to stop using them: http://entityframework.codeplex.com/wikipage?title=Design%20Meeting%20Notes%20-%20October%2025%2c%202012 We talked to the Code Contracts people at the time and didn't really get any push back about us stopping using them.

@mwwhited
Copy link
Author

That's fair... but pushing these points to the CC team could get them to make the code faster. But being that CC Tooling is out of band from VS not many people use it anyway (which is unfortunate).

It would be nice if Microsoft would actually dogfood more of their own products like CC and get them to be main stream instead of just scoffing at the idea and letting them die on the vine.

@mwwhited
Copy link
Author

BTW, it says you would discuss with the CC team. It doesn't say you would stop using them. Either way why would you use an external program that is 100% unrelated to MS to build something this core.

@rowanmiller
Copy link
Contributor

Closing as we carefully evaluated to choice to use our current solution and concluded that it was the best option for our team.

@mwwhited
Copy link
Author

mwwhited commented Apr 9, 2021

809fe7a JetBrians annotions removed

@roji
Copy link
Member

roji commented Apr 9, 2021

For anyone still interested, EF Core is now fully annotated with C# nullable reference types, obviating a lot of the use for Code Contracts.

@ajcvickers ajcvickers added closed-no-further-action The issue is closed and no further action is planned. customer-reported labels Nov 16, 2021
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

5 participants