The legacy GenAPI handled this case as well: https://github.com/dotnet/arcade/blob/608f3c7aed68dec1314d75601c6f143747e78cc6/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Methods.cs#L444-L445

https://github.com/dotnet/arcade/blob/ea698fd8c7d947c9fa46b6d26f61a315d2a74f16/src/Microsoft.Cci.Extensions/Extensions/MemberExtensions.cs#L152-L177

I think we should port that over here as well (consider ObsoleteAttributes that result in compiler errors).

MemberExtensions.IsObsoleteWithUsageTreatedAsCompilationError even returns false for [Obsolete(null, true)], just like Roslyn does for legacy compatibility. https://github.com/dotnet/roslyn/blob/15b43b33901c88f68ef43f8314b5a2457716780d/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs#L159-L165

@ViktorHofer

Scenario 1:
namespace Foo
{
    public class B
    {
        [System.Obsolete("Constructor is deprecated.", true)]
        public B(int p1) { }
    }

    public class C : B
    {
        /// CS7036: there is no argument given that corresponds to the required parameter p1
        public C() { }
    }
}

Scenario 2:

namespace Foo
{
    public class B
    {
        [System.Obsolete("Constructor is deprecated.", true)]
        public B(int p1) { }
    }

    public class C : B
    {
        /// CS0619: B.B(int) is obsolete.
        public C() : base(1) { }
    }
}

What is the best way to handle such scenario?

Unsure but can we just mimic the existing legacy GenAPI behavior? cc @ericstj

I'd start by saying that GenAPI doesn't need to handle a case that's impossible to represent in the product. It also shouldn't go out of its way to support something that's extremely unlikely (eg: we Obsolete(true) an API that is in active use by some up-stack component).

Worst case, if we somehow had a case of this scenario, then I would expect that we'd need to workaround it by having an internal constructor on the base type. Manual exclusion could also allow a user to exclude the upstack constructor if they needed to redefine it to call a non-default internal constructor. I would recommend that GenAPI omit the base call to an obsolete(true) constructor. This is what the old GenAPI did.

TL;DR

1. Omit all calls to base obsolete(true) constructors.
2. Developers can author an internal constructor on the base type manually (and a call it, if it's not default).

This comment suggests that there should be a better way to figure out if a parameter type accepts null or not: https://github.com/dotnet/arcade/blob/608f3c7aed68dec1314d75601c6f143747e78cc6/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Methods.cs#L456-L463

cc @eerhardt

same here: Use All instead of !Any for readability:

My expectation is that Any returns false when the first element found as a result in best case you don't need to traverse the whole list. On the other hand All will always traverse the list.

On the other hand All will always traverse the list.

That would be a very inefficient implementation of All. It returns as soon as it finds an element that doesn't match: https://source.dot.net/#System.Linq/System/Linq/AnyAll.cs,62

All(a => !a.IsObsoleteWithUsageTreatedAsCompilationError()) is the just the more readable statement in comparison to !Any(a => a.IsObsoleteWithUsageTreatedAsCompilationError()).

updated

We are missing a filter check here to only include constructors that are publicly accessible (i.e. filter out effectively private ones): https://github.com/dotnet/arcade/blob/608f3c7aed68dec1314d75601c6f143747e78cc6/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Methods.cs#L444-L445

members are filter out before we generate SyntaxNode out of ISymbol.

The filtering makes sure that we only traverse symbols that are included. Here we are looking at the base type constructors of the included constructor. Nothing guarantees at that point that the base type constructors are included.

You can test that with a type that has a base type with an internal ctor. This should result in a compilation error because the internal ctor of the base type isn't emitted (assuming that the IncludeVisibleOutsideOfAssembly setting is false).

Thanks, updated.