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

<Nullable> has no effect in old-style csproj #5551

Closed
jnm2 opened this issue Sep 30, 2019 · 14 comments
Closed

<Nullable> has no effect in old-style csproj #5551

jnm2 opened this issue Sep 30, 2019 · 14 comments
Labels
Legacy Issues against the legacy project system.

Comments

@jnm2
Copy link

jnm2 commented Sep 30, 2019

Visual Studio Version: 16.3.1 and 16.4p1

<Nullable>enable</Nullable> is not getting picked up by old-style csprojs. Everything works if we have to put #nullable enable in every file, but that's distasteful. Is there another workaround

We can't move to SDK-style csproj until #4938 ships in 16.4 in November.

Command-line builds show that NRT is enabled and producing the expected warnings, but the IDE is adamant that nothing is in a nullable context.

Full repro for 16.3.1 and 16.4p1:

public class C
{
    // CS8632 The annotation for nullable reference types should only be used in code within a
    // '#nullable' annotations context.
    //                  ↓
    public void M(string? nullableParameter)
    {
    }
}

Old-style csproj from new project template but with LangVersion 8.0 and Nullable enable:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  <PropertyGroup>
    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
    <ProjectGuid>0098472d-d4d6-4ea1-9a6b-142942f81782</ProjectGuid>
    <OutputType>Library</OutputType>
    <AppDesignerFolder>Properties</AppDesignerFolder>
    <RootNamespace>OldStyleCsproj</RootNamespace>
    <AssemblyName>OldStyleCsproj</AssemblyName>
    <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion>
    <FileAlignment>512</FileAlignment>
    <Deterministic>true</Deterministic>
    <LangVersion>8.0</LangVersion>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
    <DebugSymbols>true</DebugSymbols>
    <DebugType>full</DebugType>
    <Optimize>false</Optimize>
    <OutputPath>bin\Debug\</OutputPath>
    <DefineConstants>DEBUG;TRACE</DefineConstants>
    <ErrorReport>prompt</ErrorReport>
    <WarningLevel>4</WarningLevel>
  </PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
    <DebugType>pdbonly</DebugType>
    <Optimize>true</Optimize>
    <OutputPath>bin\Release\</OutputPath>
    <DefineConstants>TRACE</DefineConstants>
    <ErrorReport>prompt</ErrorReport>
    <WarningLevel>4</WarningLevel>
  </PropertyGroup>
  <ItemGroup>
    <Reference Include="System"/>
    
    <Reference Include="System.Core"/>
    <Reference Include="System.Xml.Linq"/>
    <Reference Include="System.Data.DataSetExtensions"/>
    
    
    <Reference Include="Microsoft.CSharp"/>
    
    <Reference Include="System.Data"/>
    
    <Reference Include="System.Net.Http"/>
    
    <Reference Include="System.Xml"/>
  </ItemGroup>
  <ItemGroup>
    <Compile Include="Class1.cs" />
    <Compile Include="Properties\AssemblyInfo.cs" />
  </ItemGroup>
  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
 </Project>

User Impact:

Have to choose between not using NRTs in these projects or adding #nullable enable to every source file.

@drewnoakes drewnoakes added the Legacy Issues against the legacy project system. label Oct 1, 2019
@davidwengier
Copy link
Contributor

As per Mads' blog post Building C# 8.0 C# 8 is not supported on .NET Framework, and the legacy project system only supports .NET Framework so this is not something we want to invest engineering effort into supporting.

@jnm2
Copy link
Author

jnm2 commented Oct 9, 2019

Thanks for the update. At the end of November when #4938 ships, we will be able to use SDK-style csproj.

I would like to express strong disappointment that C# 8 is not supported on .NET Framework, and with the concept that @terrajobst stated: "we don't support individual language features, we support the set." Every C# version since at least 3 has had features that only light up when certain types are available. C# 8 is not unique in this. For IAsyncEnumerable, Range, etc, it's just business as usual as with ValueTuple, FormattableString, Task, and so on. The only thing that is actually different from every previous version is Default Interface Methods which only lights up on capable .NET runtime versions.

C# 8 should be fully supported on .NET Framework, .NET Standard, and .NET Core 2.1, with the definition of full support including the specification that DIM does not light up. Here's the consequence of not defining full support this way:

  • Library authors are put in an impossible situation. If they want to respond to the growing demand to ship nullability annotations, they must either target only .NET Core 3.0 which your own guidelines correctly say is poor practice (https://docs.microsoft.com/en-us/dotnet/standard/library-guidance/) or they must use C# 8.0 on .NET Framework and Standard which you are now also saying not to do.

  • There are desktop application developers who cannot move to .NET Core until visual designers are working. You are telling us not to benefit from anything in C# 8. In twelve months, maybe we'll be able to move if the designers work and there aren't more outlying issues like Third-party Windows Forms controls that use LicenseManager crash the designer with E_NOTIMPL #4938 which require us to first discover them and then push for them ourselves.

Desktop developers may all be able to move to .NET Core in a year if we're lucky, but are you really pressuring library authors to drop support for .NET Standard, .NET Framework, and .NET Core 2.1, or else pressuring them to stay unannotated for their end users? How did you intend for this dilemma to be received?

@jnm2
Copy link
Author

jnm2 commented Oct 9, 2019

Addendum: I don't mind temporary pain. We need a sense either that you've already planned out these side effects and have specific guidance ready, or that you are willing to listen and reconsider.

The type of guidance I have in mind is not "this is hard, you're definitely on your own, figure it out" but rather e.g. "We think it's best for libraries not to ship nullability annotations until they are willing to drop support for .NET Standard, .NET Framework, and .NET Core 2.1. We think most libraries should plan on dropping this support in the .NET [5? 6? 7?] timeframe. Here's why this is a good idea for the ecosystem. [...]"

Time or budget constraints are totally legitimate things to bring up if they were a factor of any significance in the decision. That of course does not reduce the need to fully understand, own, and communicate the consequences of the decision. I want to know that nothing is being ignored and to have an utterly realistic expectation set for the path ahead.

@cartermp
Copy link
Contributor

cartermp commented Oct 9, 2019

@jnm2 There may be a statement along those lines, likely, on the .NET Blog. But the current policy is that all new features are for .NET Core 3.0, and the way that flows through tooling has ramifications like what you're mentioning here. Until then, the official statement on the matter is here: https://devblogs.microsoft.com/dotnet/net-core-is-the-future-of-net/

Beyond that, we won't be doing additional work to support a scenario in tooling that is officially unsupported.

@RudeySH
Copy link

RudeySH commented Oct 25, 2019

We've discussed this issue on StackOverflow (here) and came to the conclusion that only Visual Studio's IntelliSense does not recognize <Nullable>, but msbuild does. And, like @jnm2 also said, using #nullable enable in every file fixes the problem in Visual Studio.

Now tell me, when msbuild supports building .csproj with C# 8.0, and IntelliSense supports the use of #nullable enable, than obviously C# 8.0 is supported in .NET Framework, at least to some degree? Besides nullable reference types, I have verified that some other new features (like using declaration) also work just fine.

If we can get a complete list of what C# 8.0 features work, and what features don't work in .NET Framework, that would be much more helpful than just saying "it's not supported".

@bairog
Copy link

bairog commented Dec 2, 2019

If we can get a complete list of what C# 8.0 features work, and what features don't work in .NET Framework, that would be much more helpful than just saying "it's not supported".

Only Default interface members will not work on .NET Framework 4.8/.NET Standart 2.0.
Asynchronous streams, Indices and ranges and even nullable attributes are available via Nuget.
All other features are working (I've tested it by myself on VS 2019 16.3.10 and .NET Framework 4.8).

More info or at SO

@RudeySH
Copy link

RudeySH commented Dec 2, 2019

@bairog I read that post. It says:

Nullable reference types are also supported, but the new nullable attributes required to design the more complex nullable use cases are not. I cover this in more detail further down in the "gory details" section.

It says nullable reference types are supported (to a degree), but fails to mention that IntelliSense does not support it at all when using old-style .csproj, which makes it completely unusable when developing with Visual Studio.

Why hasn't this issue been acknowledged yet? Currently this issue is closed because "C# 8 is not supported on .NET Framework" but that's simply not true. Please reopen this issue.

@terrajobst
Copy link
Member

terrajobst commented Dec 2, 2019

@RudeySH

If we can get a complete list of what C# 8.0 features work, and what features don't work in .NET Framework, that would be much more helpful than just saying "it's not supported".

Only Default interface members will not work on .NET Framework 4.8/.NET Standard 2.0.

There is a difference between "doesn't fail" and "is supported". We don't support C# 8 on .NET Framework, period. Of course, you can force it via tooling (e.g. by specifying language version in the project file) and provided you install the right set of NuGet packages, stuff will often just work fine (like it has with past versions and bridge packages).

However, our experience with back-porting language features (such as tuples) has shown us that there is always some loss in fidelity. And we're not willing to invest the engineering resources to close whatever gap will exist to fully support C# 8 on .NET Framework. We don't test for this scenario and all NuGet packages are provided as-is, and were mostly done to unblock first parties (like the Azure SDK) to author a single NuGet package for .NET Standard 2 that has a great experience when being consumed from .NET Core 3 while still being usable from .NET Framework in C# 7.3.

@jnm2

C# 8 should be fully supported on .NET Framework, .NET Standard, and .NET Core 2.1, with the definition of full support including the specification that DIM does not light up.

We considered this but we'll have the same discussion in C# 9 and then in C# 10 and so forth. We already told people that .NET Framework is feature complete and that we'll not invest in new features. In that sense, not supporting C# 8 on .NET Framework is the natural next step. If we want to innovate in .NET Core, then we need to cut ties with .NET Framework.

To be clear, this isn't a debate on what features work or which ones don't: it's about us focusing all investment on .NET Core. And that includes testing, analyzing and planning new features for C# as well.

@bairog
Copy link

bairog commented Dec 4, 2019

It says nullable reference types are supported (to a degree), but fails to mention that IntelliSense does not support it at all when using old-style .csproj, which makes it completely unusable when developing with Visual Studio.

What do you mean? I've created new Console Application for .NET Framework 4.0 with VS 16.3.10 (old-style csproj) and then followed this guide for Nullable Nuget package. And now I have compler warning when trying to set null for not-null field and also have IntelliSense and highlighting for AllowNullAttribute .

We don't support C# 8 on .NET Framework, period.
And we're not willing to invest the engineering resources to close whatever gap will exist to fully support C# 8 on .NET Framework.
We considered this but we'll have the same discussion in C# 9 and then in C# 10 and so forth.

MS intentions are clear to me and my company. But for today our customers still have Windows XP support requirement ( .NET 4.0 arrgh.. :( ) and we simply want to have most modern IDE and C# language version that is possible. So we use this technique with specifying language version in the project file and third-party Nuget packages.

@swythan
Copy link

swythan commented Dec 6, 2019

@terrajobst

all NuGet packages are provided as-is, and were mostly done to unblock first parties (like the Azure SDK) to author a single NuGet package for .NET Standard 2 that has a great experience when being consumed from .NET Core 3 while still being usable from .NET Framework in C# 7.3.

Both here and in Mads' blog post about NRT adoption it is acknowledged that there is no way to produce a NuGet package with nullable annotations without either dropping .Net Framework support, or compiling in this "C#8-on-NetFx" configuration which you are so keen to point out is unsupported.

  • Is it reasonable to be proposing that most NuGet packages (including 1st party ones) published in the next year should be compiled in this unsupported configuration?
  • Should I be expecting all my dependencies to arrive in this form if I'm still working on .Net Framework? (spoiler: I am, because we use WCF.)

@terrajobst
Copy link
Member

There are two different things here:

  1. Consuming C# 8 to build a null-annotated .NET Standard 2.0 library.
  2. Consuming C# 8 in .NET Framework.

(1) We will support library authors using C# 8 to build null-annotated libraries targeting .NET Standard 2.0. However, even in .NET Standard, this doesn't make all of C# 8 supported, only this particular subset. The goal of this effort is to allow library developers to have a single binary that they can use from .NET Standard 2.1/.NET Core 3.0 and see null annotations without having to give up .NET Framework support.

(2) But we still don't support consuming null annotations from .NET Framework.

@bairog
Copy link

bairog commented Dec 7, 2019

@terrajobst

(1) We will support library authors using C# 8 to build null-annotated libraries targeting .NET Standard 2.0. ...
...The goal of this effort is to allow library developers to have a single binary that they can use from .NET Standard 2.1/.NET Core 3.0 and see null annotations without having to give up .NET Framework support.

(2) But we still don't support consuming null annotations from .NET Framework.

Could you clarify more, please:

  • If I use nullable attributes directly in my .NET Framework application - that is not supported.
  • If I use nullable attributes in my .NET Standart 2.0 library, produce Nuget package and then add reference to that package in my .NET Framework application - that is supported.

Does that make sense?

@swythan
Copy link

swythan commented Dec 9, 2019

@terrajobst

(1) We will support library authors using C# 8 to build null-annotated libraries targeting .NET Standard 2.0. However, even in .NET Standard, this doesn't make all of C# 8 supported, only this particular subset.

This disagrees with your previous statement:

There is a difference between "doesn't fail" and "is supported". We don't support C# 8 on .NET Framework, period.

and also disagrees with @MadsTorgersen's bog post:

You also have to set the language version to C# 8.0, of course, and that is not a supported scenario when one of the target versions is below .NET Core 3.0.

Can you see why this is confusing?

I've been looking for a page where the support policy is actually stated, but couldn't find one. I did find this github issue, though, which may be a better place to have this discussion:
dotnet/csharplang#2651

@cartermp
Copy link
Contributor

cartermp commented Dec 9, 2019

@swythan the documentation is here: https://docs.microsoft.com/dotnet/csharp/language-reference/configure-language-version

I recommend we halt conversation in this issue, since it is not about the general support of C# 8, and there will be no further tooling support in the legacy project system to enable the <Nullable> property.

@dotnet dotnet locked as off-topic and limited conversation to collaborators Dec 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Legacy Issues against the legacy project system.
Projects
None yet
Development

No branches or pull requests

8 participants