Nullable for the solution

AArnott / CodeGeneration.Roslyn

Assists in performing Roslyn-based code generation during a build.

Microsoft Public License

408 stars 60 forks source link

Nullable for the solution #225

Closed manne closed 4 years ago

manne commented 4 years ago

Implies usage of C# 8.0, instead of 7.3

manne commented 4 years ago

see #184

manne commented 4 years ago

I did not make the command line functionality nullable aware, as it has a "huge" complexity and some functionality which is not being used. Is the source of command line functionality this repo dotnet/command-line-api? Unfortunately that code base is not nullable aware.

manne commented 4 years ago

Changing API shape (CompilationGenerator) also doesn't quite feel like it belongs in here. But that I'll leave up to @AArnott.

I have no problem to change it back,

I think, if this PR is true to it's name, should be enabled in Directory.Build.props and then in projects that weren't/couldn't be migrated, disable it. Currently that'll be only Tests and Tool it seems?

I activated nullable globally. I found some interesting situations 🙂

manne commented 4 years ago

To get rid of the build error

we could update the nuget packge StyleCop.Analyzers to 1.2.0-beta.164. see dotnetanalyzers/stylecopanalyzers#3098 or
suppress the warning or
disable nullable for the corresponding project

AArnott commented 4 years ago

Once NRT is activated, I omit/remove null checks on all internal methods. But as @amis92 says, I keep null checks present on public entrypoints.