praeclarum / FuGetGallery

An alternative web UI for browsing nuget packages
https://www.fuget.org
MIT License
683 stars 121 forks source link

Added capability to show obsolete members in diff with message #171

Open jamesmcroft opened 2 years ago

jamesmcroft commented 2 years ago

A look at implementing a fix for #167, providing diff information for obsolete members.

Change includes extra NumObsolete fields to the existing diff info types and an ObsoleteMessage field to the MemberDiffInfo type.

Updates made to the diff UI to reflect obsolete members showing the message

mindsolve commented 2 years ago

Hi, thank you for that change! This is going to be helpful.

I have just tested the changes and have run into an exception in your code. My assumption is that the code doesn't handle ObsoleteAttribute calls without arguments.

The package MineStat marked multiple methods as obsolete from 1.x->2.0.0, but did not provide workaround messages (only added in 2.1.1). Comparison between 1.x and 2.1.1 work flawlessly, listing the added deprecations without error.

Stacktrace:

InvalidOperationException: Sequence contains no elements
System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at FuGetGallery.ApiDiff.ValidateObsolete(ICustomAttributeProvider member) in <$ProjectDir>\FuGetGallery\Data\ApiDiff.cs:line 223
   at FuGetGallery.ApiDiff..ctor(PackageData package, PackageTargetFramework framework, PackageData otherPackage, PackageTargetFramework otherFramework) in <$ProjectDir>\FuGetGallery\Data\ApiDiff.cs:line 174
   at FuGetGallery.ApiDiff.ApiDiffCache.<>c__DisplayClass1_0.<GetValueAsync>b__0() in <$ProjectDir>\FuGetGallery\Data\ApiDiff.cs:line 249
   at System.Threading.Tasks.Task`1.InnerInvoke()
   at System.Threading.Tasks.Task.<>c.<.cctor>b__272_0(Object obj)
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)

To replicate: call the URL /packages/MineStat/2.0.0/lib/net46/diff/1.0.1/ (Do an API comparison between versions 1.0.1 and 2.0.0 of the package MineStat)

(edit: forgot to include the actual error message 😉)

jamesmcroft commented 2 years ago

Thanks for the example, I tried with a few that I'd known and all came out okay 😅

I'll take a look 😄

jamesmcroft commented 2 years ago

@mindsolve that should be solved now. Thanks for testing this 👍🏻

TylerBrinkley commented 2 years ago

Does this show obsolete messages in a non-diff context viewing of the API?

mindsolve commented 2 years ago

@TylerBrinkley: No, it doesn't. I have implemented that feature in my branch based on this PR, so either @jamesmcroft integrates it in here or I'll open a second PR after this one has been merged :) (#173)

jamesmcroft commented 2 years ago

@mindsolve @TylerBrinkley happy to look into the non-diff context as part of this change 👍🏻

jamesmcroft commented 2 years ago

Merged the changes in that @mindsolve added.

This PR should now cover API diff obsolete messaging as well as the non-diff context @TylerBrinkley 👍🏻