Xmldoc with <see cref="Foo{T}(T?)"/> or similar fails to compile

[cyanite]

Visual Studio 2019 RC (csc.exe version 3.0.19.12206 (ec36668)):

Steps to Reproduce:

Write this code. Note that Visual Studio helps you complete the <see cref...>. Enable XML documentation file and enable all warnings as errors.

using System;
namespace Repro2
{
    /// <summary/>
    public class Class1
    {
        void Foo<T>(T t) where T : class { }
        void Foo<T>(T? t) where T : struct { }

        /// <summary>
        /// See <see cref="Foo{T}(T?)"/>
        /// </summary>
        void Bar() { }
    }
}

Expected Behavior:
It works.

Actual Behavior:
It fails with CS1580 Invalid type for parameter T? in XML comment cref attribute: 'Foo{T}(T?)'

If you change it to <see cref="Foo{T}(Nullable{T})"/> it works, but Visual Studio will now nag you to change it back to the version that doesn't :p.

This is a regression; both syntaxes worked in Visual Studio 2017. The old compiler produced the following in the document file:

<see cref="M:Repro2.Class1.Foo``1(System.Nullable{``0})"/>

The new one produces:

<see cref="M:Repro2.Class1.Foo``1(``0)"/>

(and warns/fails if warnings as errors is enabled.)

[gafter]

It appears the addition of the nullable ref types feature may have introduced a regression here.

[jcouv]

Confirmed this is a regression (15.9 accepts this xml doc). Investigating....

[jcouv]

Fixed for dev16 (rc4)

[jcouv]

Thanks @cyanite for reporting this!

[branko-d]

Fixed for dev16 (rc4)

Not fixed in Visual Studio 2019 Professional RTM (version 16.0.0).

[jcouv]

@branko-d I've tried the code in OP in 16.0 preview 5 (which is very close to RTM) just now, and it compiled fine.

@jaredpar Could someone try to repro on dev16.0 RTM bits while I'm away?

[jcouv]

My bad. It does repro on 16.0 preview 5. I forgot to enable XML documentation in the project.
That said, the test I had added seems correct. Needs some investigation.

[RikkiGibson]

I can't repro this in 16.3 preview 3.

[jaredpar]

@RikkiGibson great. Can we change the test @jcouv originally added so that it forces XML doc creation? That should close the gap.

[branko-d]

Sorry for nagging but...

I'm still seeing the issue in Visual Studio Professional 2019 16.3.1.

[jaredpar]

This does not repro with latest

[daiplusplus]

@jaredpar I can reproduce this in a .NET Framework 4.7.2 project with C# 7.3 in Visual Studio 2019 16.7.2. Code and screenshot below.

The difference seems to be my use of a nullable generic enum type parameter. Should I file a new bug or will you reopen this issue?

Relevant code:

		/// <summary>Combines <see cref="CleanPagingParams(int, string, int?, int?, out IReadOnlyList{string}, out int, out int)"/> and <see cref="CleanOrderByParams{TOrderByEnum}(TOrderByEnum?, bool?, out TOrderByEnum, out bool)"/>.</summary>
		public static ActualPagingParams<TOrderByEnum> CleanAllParams<TOrderByEnum>( Int32 defaultPageSize, IHasPagingParams<TOrderByEnum> paras, out IReadOnlyList<String> terms, out Int32 pageIndex, out Int32 pageSize, out TOrderByEnum orderBy, out Boolean asc )
			where TOrderByEnum : struct, Enum
		{
			CleanPagingParams(
				defaultPageSize: defaultPageSize,
				search         : paras.Search,
				pageNumber     : paras.PageNumber,
				count          : paras.PageSize,
				terms          : out terms,
				pageIndex      : out pageIndex,
				pageSize       : out pageSize 
			);

			CleanOrderByParams<TOrderByEnum>(
				inputOrderBy   : paras.OrderBy,
				inputOrderByAsc: paras.OrderByAsc,
				orderBy        : out orderBy,
				asc            : out asc
			);

			return new ActualPagingParams<TOrderByEnum>( terms: terms, orderBy: orderBy, defaultPageSize: defaultPageSize, pageIndex: pageIndex, pageSize: pageSize, orderByAsc: asc );
		}

		public static void CleanOrderByParams<TOrderByEnum>( TOrderByEnum? inputOrderBy, String inputDir, out TOrderByEnum orderBy, out Boolean asc )
			where TOrderByEnum : struct, Enum
		{
			if( inputOrderBy.HasValue && EnumTraits<TOrderByEnum>.IsDefined( inputOrderBy.Value ) )
			{
				orderBy = inputOrderBy.Value;
			}
			else
			{
				orderBy = EnumOrderBy<TOrderByEnum>.DefaultOrderBy;
			}

			CleanOrderByAscParams( orderBy, inputDir, out asc );
		}

Screenshot:

[RikkiGibson]

It would be really helpful to have a more minimal reproducer. I wasn't able to come up with one in a few minutes of trying. I defer to @jaredpar on whether this issue should be reopened or a new issue should be opened.

[jaredpar]

I agree we'd need more info in order to make progress here. The code samples we have no longer reproduce this behavior hence there really isn't a way to make progress without a new one that demonstrates the negative behavior.

[RikkiGibson]

I found a minimal reproducer. It seems that having a LangVersion < 8 is key.

Project.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>library</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
    <LangVersion>7.3</LangVersion>
    <DocumentationFile>ConsoleApp11.xml</DocumentationFile>
  </PropertyGroup>
</Project>

Program.cs

/// <summary>
/// Something about C
/// </summary>
public class C
{
    // warning CS1580: Invalid type for parameter T? in XML comment cref attribute: 'M{T}(T?)'
    /// <summary>Something about <see cref="M{T}(T?)" /></summary>
    public static void M<T>(T? t) where T : struct
    {
    }
}

[daiplusplus]

BTW, there exists a workaround: manually change T? to Nullable{T} and then the warnings go away.

So if you have:

public static String Foo<TValue>(this TValue? value)
    where TValue : struct, IConvertible
{
}

/// <summary>Alias for <see cref="Foo{TValue}(TValue?)"/>.</summary>
public static String Bar<TValue>(this TValue? value)
    where TValue : struct, IConvertible
{
}

Change it to this:

/// <summary>Alias for <see cref="Foo{TValue}(Nullable{TValue})"/>.</summary>
public static String Bar<TValue>(this TValue? value)
    where TValue : struct, IConvertible
{
}

and the warning is solved. No need for suppression. Phew!

[RikkiGibson]

It's good to hear you were able to work around the issue. Apparently I found a reproducer that works with old LangVersion, but I neglected to reopen the issue. Sorry about that--it does feel like we should address this problem.