microsoft / CodeContracts

Source code for the CodeContracts tools for .NET
Other
882 stars 151 forks source link

Code Contracts v.1.10 #297

Open SergeyTeplyakov opened 8 years ago

SergeyTeplyakov commented 8 years ago

Hello Everyone,

I think it's time to release new version of this tool. We've made a lots of changes that would be very useful for customers.

There is a tons of existing issues. Many of them are fixed but appropriate PR's are not merged yet (I'm working on it right now). Some issues are related to static checker and I don't know when they would be addressed. And there are few issues that I would like to fix before this upcoming release (for instnace, #242).

Here I would like to discuss and prioritize existing issues that needs to be fixed before releasing new version to public audience. If yout think that some of them are critical, feel free to put a comment here.

Feedback is higly wellcome and appreciated!

P.S. We've made a great job fixing tons of issues, I highly appreciated all the effort that we've made so far!

yaakov-h commented 8 years ago

FWIW, in order to move my team to VS2015 I've had to maintain a private fork of 1.9.1 plus a handful of small fixes, cherry-picked from master.

I did try to use master at first, but encountered many issues, including a bunch of contract failures on Expression when the compiler turned LINQ into code. I'd assume it was caused by #171, but haven't done a detailed check yet.

There are also a handful of issues with Roslyn which required suppressing a couple hundred Contracts warnings across a large codebase, #205 being the largest common pattern.

SergeyTeplyakov commented 8 years ago

@yaakov-h I would really glad to get more details on the master state.

I didn't have a chance to look into static in more details, so I have no ideas what the effort is required to fix #205.

I would happy to hear some feedback about ccrewrite as well, because it seems that main issues that you've encounted are related to static checker.

P.S. I would happy to help with #171, but I need some additional information. If you would be able to file some repro or open a bug for this, it would be really helpful.

yaakov-h commented 8 years ago

If I have some time later this week or early next, I'll check up on master again and see how it fares across my team project. I'd take the async stuff for a long run too, but we stopped contracting async methods due to the various issues with it.

SergeyTeplyakov commented 8 years ago

Thanks a lot! Will wait for your feedback. BTW, I would really like to get your feedback on new async stuff. There was a tons of changes in this area and after mergint latest stuff (that I'm going to do tomorrow), it should be in a much better shape. I'm sure there are some other issues, but I would like to know about them to fix.

yaakov-h commented 8 years ago

Haven't checked runtime (i.e. that the rewriter is working fully) or async yet, but I made it just under 40% of the way through my project with 0f817e7 until I hit #301 as mentioned above.

SergeyTeplyakov commented 8 years ago

@yaakov-h Thanks a lot for your feedback about static checker. It seems that I need to take a close look into that stuff. If you would have some time I still would be very greatful about your feedback regarding ccrewrite.

yaakov-h commented 8 years ago

I won't have a chance to check that until early/mid-next week.

schlumpfmarkus commented 8 years ago

Hello Everyone,

I think Sergey is absolutely right! We urgently need a current release to be able to really use code contracts with VS2015 and Roslyn! As the release date will have an impact on our projects, can you roughly estimate, when the new release should be available?

Anyhow, thank you all for your hard work!!

Regards, Markus

SergeyTeplyakov commented 8 years ago

I would say that week or two is a reasonable timeline.

The only question is scope. If we think that we need to to fix cccheck/ccrefgen before next release, it would be much longer. But currently I'm thinking to focus in this release primarily on ccrewrite/installer and other issues and fix ccrech/ccrefgen in the next release (just because I never worked on that areas so it will take some time to fix them).

maw136 commented 8 years ago

Hello,

I concur with schlumpfmarkus and also support Sergey in that the ccrewrite/installer is much more important now than static checker - especially the null pointer problems with lambda/async.

krk commented 8 years ago

FWIW, StackOverflowException in https://github.com/Microsoft/CodeContracts/issues/169 affects code contracts editor extension too.

krk commented 8 years ago

Also, what happens to the crash reports submitted via this dialog:


Sorry!

An exception was thrown while the Code Contracts Editor Extension was running. Please assist our development team by sending the crash details.

Send crash details?

Yes No Cancel

ndykman commented 8 years ago

I agree completely that a release that focuses on ccrewrite and the installer is completely merited.

Also, I am wondering how to best engage the larger community around advocating for this tool. I've already commented on the .Net core site that the need for adding T? and T! for null is already handled much more effectively by CodeContracts, which allows one to state that at this time, this can't be null.

yaakov-h commented 8 years ago

I do also agree, but given the issues with the static checker in Roslyn-generated code I don't think 1.10 (or 1.9.1 for that matter) should be considered VS2015-compatible.

@ndykman I'm not sure this needs advocacy. It's a tool, not a religion.

@SergeyTeplyakov anything in particular I should check w.r.t ccrewrite on master?

aarondandy commented 8 years ago

I noticed on the gallery page somebody is having issues with the VS extension:

Current package seems to have a problem with Visual Studio 2015. VS does not start after this package is installed. The splash screen comes up and then it goes away and the process is gone. The error received is 'No InprocServer32 registered for package [Async Query Service Package]'. A repair install of VS is required to fix.

No idea if that would be addressed in 1.10 but people are definitely having problems with the current release.

Would it be a better idea to get these releases flagged as beta and some of the older ones as well? Nobody likes to know that something they already installed was downgraded to a "beta" after the fact but I would think that would be better than pretending that things are OK. This would allow more frequent releases for those who are willing to tolerate something that is not stable.

asvishnyakov commented 8 years ago

@ndykman @yaakov-h Language support for Code Contracts may radically reduce numbler of lines of code and improve code readability, but Roslyn team do nothing to get closer to goal. So this needs advocacy.

aarondandy commented 8 years ago

@asvishnyakov When I read the github thread on method contracts in C# 7 (?) and also after talking to other people offline about it I'm pretty convinced that it won't be happening unless somebody external offers up a PR for it.

rkeithhill commented 8 years ago

I agree completely that a release that focuses on ccrewrite and the installer is completely merited.

Ditto. Any word on an ETA for v.1.10?

yaakov-h commented 8 years ago

@SergeyTeplyakov So is anything actually happening with this?

trzombie commented 8 years ago

I'm also interested in a VS2015 compatible build.

SergeyTeplyakov commented 8 years ago

@yaakov-h Sorry for the delay. I've merged all relevant PRs (one is left, working on the merge right now) and after that I'll prepare beta release.

yaakov-h commented 8 years ago

Thanks @SergeyTeplyakov.

Can we do anything about #301? That's going to be a blocker for my teams upgrading to a new release, and I'd rather not maintain a private fork that just rolls back #171.

I already have a private fork that pulls in a few VS2015-specific fixes on top of 1.9.10714.2, and I'd like to be able to drop the fork when a new release is published.

Also, what about the various PRs to add and improve contracts for system libraries?

SergeyTeplyakov commented 8 years ago

We can roll back entire #171 but maybe it would be good to narrow an issue down and update contracts properly?

Also, what about the various PRs to add and improve contracts for system libraries?

My main concern with this, is potential issues similar to #301. Because static checker is a tricky beast, merging those PR's could change CCCheck behavior in some unpredictible way. I'm OK to merge them, but I need some guidance in terms of whether they're good or not. I'm not using cccheck in any of my projects, so I don't have enough experience or appropriate codebases for testing. If I can ask you to validate those PRs I would happy to accept them!

yaakov-h commented 8 years ago

@SergeyTeplyakov if we can do something about #13, then I could likely use my own (closed-source) codebase to test some of the contract PRs without the maintenance headache and interruption of having to upgrade/downgrade constantly across a range of machines.

I'd love to be able to continue with #171 but I have no idea where to even begin looking in the static checker to fix #301 - I assume it needs to be taught some assumptions about compiler-generated code.

SergeyTeplyakov commented 8 years ago

As per #13, I'm going to publish official prerelease version of DotNet.CodeContracts package today.

And regarding #171. Your main issue is in this line?:

C:\Temp\ContractExpressionDemo\ContractExpressionDemo\Program.cs(18,13): warning : CodeContracts: requires unproven: !field.IsStatic || expression == null

yaakov-h commented 8 years ago

Yes.

SergeyTeplyakov commented 8 years ago

Ok. I'll publish nuget package and create first beta of 1.10 and will take a look at #171.

SergeyTeplyakov commented 8 years ago

Create a release: https://github.com/Microsoft/CodeContracts/releases/tag/v1.10.10126.2

And would be really grateful for the feedback!

P.S. Thanks a lot for all your contributions, that was amazing results by all of you!

pculver commented 8 years ago

Any update on when 1.10 will be released?

Worthaboutapig commented 8 years ago

Bump. It's over 6 months now since this was raised, 4 months and 125 commits since the RC.

I really appreciate the work done and don't want to do without CC, but like others, if it's not being supported, then it's worse than not having it at all.

Is there a schedule for release, what needs fixing before release etc? No milestone attached.

yaakov-h commented 8 years ago

I've given up waiting for an official release and taken to just compiling my own. 😞

Current master branch is almost usable.

pgeerkens commented 8 years ago

I have a small Monad library with usage examples that relies on CCCheck to statically verify the absence of null references. This project is welcome to use it as part of a test suite for CCCheck.exe. https://github.com/pgeerkens/Monads

Worthaboutapig commented 8 years ago

Well, I'm using the RC and it crashes with a null reference exception 4/5 times, but generally compiles on the second attempt. I noticed it did this some months ago when a colleague used it.

yaakov-h commented 8 years ago

@Worthaboutapig Would that be the same null ref as #366/#371? I've never seen this issue in the wild myself, but that fix has been merged into master now.

Worthaboutapig commented 8 years ago

@yaakov-h Yup, same stack trace. Avoided it before by turning of the contract documentation, I think. I'll try and find some time to pull master and create a build and test it, unless there's a current msi of master I could use? (pretty-please :) )

yaakov-h commented 8 years ago

It's not master, but here's the MSI from that pull request.

Worthaboutapig commented 8 years ago

Thanks, I'll attempt my own build, if that fails, I'll use yours.

danielcweber commented 8 years ago

Has anyone figured out how to make cccheck part of the build when only the nuget package is added to the project (i.e. the msi is not on the machine, like on a build server)? Last time I checked I found that the build step is not added automatically. I'm especially interested on getting this done for xproj/project.json projects.

yaakov-h commented 8 years ago

@danielcweber It works for me by enabling static analysis in the project, and then doing this in the csproj file around Microsoft.CSharp.targets

<PropertyGroup>
    <DontImportCodeContracts>True</DontImportCodeContracts>
</PropertyGroup>
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
<!-- Code Contracts -->
<PropertyGroup>
    <CodeContractsInstallDir>Path\To\DotNet.Contracts\</CodeContractsInstallDir>
</PropertyGroup>
<Import Project="$(CodeContractsInstallDir)\MsBuild\v$(MSBuildToolsVersion)\Microsoft.CodeContracts.targets" />

You'll have to adjust CodeContractsInstallDir as appropriate.

SergeyTeplyakov commented 8 years ago

@danielcweber I'm not sure about xproj/project.json support right now when ASP.NET team just announce that it moving back to msbuild.

P.S. I hope that suggestion that @yaakov-h made will help you to resolve your issues.

SergeyTeplyakov commented 8 years ago

@yaakov-h Is there anything pending for next release? I was out of this stuff and don't want to miss anything. But if we're ok, tomorrow I'll prepare next release.

yaakov-h commented 8 years ago

@SergeyTeplyakov master seems to be ok. I'm testing now, I'll find out for sure in about an hour.

I don't think there's anything else pending.

yaakov-h commented 8 years ago

@SergeyTeplyakov All OK. :white_check_mark:

pculver commented 7 years ago

Any updated on moving out of RC and to an official release?