stakx / TypeNameFormatter

A small .NET library for formatting type names à la C#.
MIT License
33 stars 4 forks source link

Disable nullable context in the file #43

Closed kzu closed 4 years ago

kzu commented 4 years ago

Workaround for #42 by just disabling nullable context entirely for the entire file.

Potential improvement would be to actually properly annotate with nullable and fix whatever nullable errors are raised after enabling it.

kzu commented 4 years ago

Hm... I guess it's not as simple. This will not build (apparently) on MSBuild15/VS2017 and prior :(

stakx commented 4 years ago

@kzu I ran a quick experiment by replacing the Copy task in the .targets file:

https://github.com/stakx/TypeNameFormatter/blob/08a2de8779180c4a6b7d70ab7642d381f95da1bd/src/TypeNameFormatter/TypeNameFormatter.Sources.targets#L33

with a combination of ReadLinesFromFile and WriteLinesToFile:

    <WriteLinesToFile File="$(BaseIntermediateOutputPath)TypeNameFormatter.cs" Lines="" Overwrite="True" />
    <WriteLinesToFile File="$(BaseIntermediateOutputPath)TypeNameFormatter.cs" Condition="'$(TypeNameFormatterNullableDisable)' == 'True'" Lines="#nullable disable" Overwrite="False" />
    <ReadLinesFromFile File="$(SourceFolder)TypeNameFormatter.cs">
      <Output TaskParameter="Lines" ItemName="TypeFormatterLinesOfCode" />
    </ReadLinesFromFile>
    <WriteLinesToFile File="$(BaseIntermediateOutputPath)TypeNameFormatter.cs" Lines="@(TypeFormatterLinesOfCode)" Overwrite="False" />

and I added a new property <TypeNameFormatterNullableDisable> (defaults False) that you can set to True in your project file.

It works... sort of. There are two annoying details that make this a less-than-ideal solution:

Update 1:

WriteLinesToFile completely destroys indentation.

Instead of using WriteLinesToFile, use an inline code task:

-<Copy SourceFiles="$(SourceFolder)TypeNameFormatter.cs" DestinationFolder="$(BaseIntermediateOutputPath)" OverwriteReadOnlyFiles="true" /> 
+<TypeNameFormatterGenerateSourceFile InputFile="$(SourceFolder)TypeNameFormatter.cs" OutputFile="$(BaseIntermediateOutputPath)TypeNameFormatter.cs" EmitNullableDisable="$(TypeNameFormatterNullableDisable)" />

defined as follows:

  <UsingTask TaskName="TypeNameFormatterGenerateSourceFile"
             TaskFactory="CodeTaskFactory"
             AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll" >
    <ParameterGroup>
      <EmitNullableDisable ParameterType="System.Boolean" Required="True" />
      <InputFile ParameterType="System.String" Required="True" />
      <OutputFile ParameterType="System.String" Required="True" />
    </ParameterGroup>
    <Task>
      <Using Namespace="System" />
      <Using Namespace="System.IO" />
      <Code Type="Fragment" Language="Cs">
        <![CDATA[
          string contents = File.ReadAllText(InputFile);
          if (EmitNullableDisable)
          {
            contents = "#nullable disable\r\n\r\n" + contents;
          }
          File.WriteAllText(OutputFile, contents);
        ]]>
      </Code>
    </Task>
  </UsingTask>

I still have no idea about the file not being regenerated when <TypeNameFormatterNullableDisable> would be changed after the package restore.

kzu commented 4 years ago

Interesting. I think I would default the property to true if $(Nullable)!='disable'. It's counter-intuitive that you're disabling nullable in the file if nullable is not disabled in the project, but that's precisely what you need to do anyway 🤔

stakx commented 4 years ago

What I'm not entirely clear about is how to determine in MSBuild whether or not the compiler supports #nullable. What if <Nullable> is simply not set at all? Does one query <LangVersion> next? Or the MSBuild tools version or install path? Or something else altogether?

kzu commented 4 years ago

I'm pretty sure unless you have a non-empty value for $(Nullable), it will not be turned on for the compiler. I think it's safe to assume that if you set some value there, it's because you have a nullable-capable compiler, right?

stakx commented 4 years ago

Yes, that makes sense. Let's hope it really is that simple for a change.

How about we duplicate the main code file and add #nullable disable there (or even better, #nullable enable and actual nullable annotations)? Then we choose the file to be copied based on whether or not <Nullable> has a value.

I hate the duplicated code, but I think it's better and more flexible than the MSBuild contortion I demonstrated above.

For now that would be good enough for me. For the longer term, I'll look into some other possibilities (such as using T4 templates to generate the source file, and querying MSBuild properties from it, if possible).

kzu commented 4 years ago

I think the simpler the better. I'm done with T4, I love Scriban now :)

stakx commented 4 years ago

Closing this PR in favor of #44.