Skip to content
Permalink
Browse files

Report bad tightening/loosening of attributes in OHI (#41336)

  • Loading branch information
jcouv committed Feb 12, 2020
1 parent 35e23e1 commit 961f261042c7c9326d38bc35da944b693869a50c
Showing with 2,923 additions and 196 deletions.
  1. +13 −0 docs/compilers/CSharp/Compiler Breaking Changes - post VS2019.md
  2. +18 −1 docs/features/nullable-reference-types.md
  3. +132 −0 src/Compilers/CSharp/Portable/CSharpResources.Designer.cs
  4. +42 −0 src/Compilers/CSharp/Portable/CSharpResources.resx
  5. +10 −0 src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
  6. +17 −0 src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
  7. +126 −1 src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
  8. +7 −0 src/Compilers/CSharp/Portable/Generated/ErrorFacts.Generated.cs
  9. +63 −27 src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs
  10. +1 −1 src/Compilers/CSharp/Portable/Symbols/Source/SourceOrdinaryMethodSymbol.cs
  11. +16 −8 src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
  12. +1 −1 src/Compilers/CSharp/Portable/Symbols/TypeWithState.cs
  13. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
  14. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
  15. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
  16. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
  17. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf
  18. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf
  19. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf
  20. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf
  21. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf
  22. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf
  23. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf
  24. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf
  25. +70 −0 src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf
  26. +1,534 −148 src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
  27. +7 −0 src/Compilers/CSharp/Test/Syntax/Diagnostics/DiagnosticTest.cs
  28. +3 −1 src/Compilers/Core/Portable/CodeGen/PrivateImplementationDetails.cs
  29. +1 −1 src/Compilers/Core/Portable/CommandLine/SarifDiagnosticComparer.cs
  30. +1 −1 src/Compilers/Core/Portable/Diagnostic/CommonDiagnosticComparer.cs
  31. +6 −1 src/Compilers/Core/Portable/InternalUtilities/SpecializedCollections.Empty.Dictionary.cs
  32. +3 −1 src/Compilers/Core/Portable/PEWriter/MemberRefComparer.cs
  33. +3 −1 src/Compilers/Core/Portable/PEWriter/MethodSpecComparer.cs
  34. +2 −1 src/Compilers/Core/Portable/PEWriter/TypeSpecComparer.cs
  35. +6 −1 src/Compilers/Core/Portable/SourceFileResolver.cs
  36. +1 −1 src/Compilers/Core/Portable/SwitchConstantValueHelper.cs
@@ -108,3 +108,16 @@ Similarly, a value from a `[DisallowNull] ref string? p` parameter will be assum
On the other hand, we'll warn for returning a `null` from a method declared with `[NotNull] string?` return type, and we'll treat the value from a `[AllowNull] ref string p` parameter as maybe-null.
Conditional attributes are treated leniently. For instance, no warning will be produced for assigning a maybe-null value to an `[MaybeNullWhen(...)] out string p` parameter.

19. https://github.com/dotnet/roslyn/issues/36039 In *Visual Studio 2019 version 16.3* and onwards, the compiler did not check the usage of nullable flow annotation attributes, such as `[MaybeNull]` or `[NotNull]`, in overrides or implementations. In *Visual Studio 2019 version 16.5*, those usages are checked to respect null discipline. For example:
``` csharp
public class Base<T>
{
[return: NotNull]
public virtual T M() { ... }
}
public class Derived : Base<string?>
{
public override string? M() { ... } // Derived.M doesn't honor the nullability declaration made by Base.M with its [NotNull] attribute
}
```

@@ -99,7 +99,24 @@ The `Interlocked.CompareExchange` methods have special handling in flow analysis

When simple pre- and post-condition attributes are applied to a property, and an allowed input state is assigned to the property, the property's state is updated to be an allowed output state. For instance an `[AllowNull] string` property can be assigned `null` and still return not-null values.

The use of any member returning `[MaybeNull]T` for an unconstrained type parameter `T` produces a warning, just like `default(T)` (see below). For instance, `listT.FirstOrDefault();` would produce a warning.

Enforcing nullability attributes within method bodies:
- An input parameter marked with `[AllowNull]` is initialized with a maybe-null (or maybe-default) state.
- An input parameter marked with `[DisallowNull]` is initialized with a not-null state.
- A parameter marked with `[MaybeNull]` or `[MaybeNullWhen]` can be assigned a maybe-null value, without warning. Same for return values. Same for a nullable parameter marked with `[NotNullWhen]` (the attribute is ignored).
- A parameter marked with `[NotNull]` will produce a warning when assigned a maybe-null value. Same for return values.
- The state of a parameter marked with `[MaybeNullWhen]`/`[NotNullWhen]` is checked upon exiting the method instead.
- A method marked with `[DoesNotReturn]` will produce a warning if it returns or exits normally.

Note: we don't validate the internal consistency of auto-properties, so it is possible to misuse attributes on auto-props as on fields. For example: `[AllowNull, NotNull] public TOpen P { get; set; }`.

Enforcing nullability attributes when overriding and implementing:
In addition to checking the types in overrides/implementations are compatible with overridden/implemented members, we also check that the nullability attributes are compatible.
- For input parameters (by-value and `in`), we check that the widest allowed value by the overridden/implemented parameter can be assigned to the overriding/implementing parameter.
- For output parameters (`out` and return values), we check that the widest produced value by the overriding/implementing parameter can be assigned to the overridden/implemented parameter.
- For `ref` parameters and return values, we check both.
- We check that the post-condition contract `[NotNull]`/`[MaybeNull]` on input parameters is enforced by overriding/implementing members.
- We check that overrides/implementations have the `[DoesNotReturn]` attribute if the overridden/implemented member has it.

## `default`
If `T` is a reference type, `default(T)` is `T?`.

Some generated files are not rendered by default. Learn more.

@@ -5339,12 +5339,24 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_NullabilityMismatchInReturnTypeOnOverride_Title" xml:space="preserve">
<value>Nullability of reference types in return type doesn't match overridden member.</value>
</data>
<data name="WRN_NullabilityMismatchInReturnTypeOnOverrideBecauseOfAttributes" xml:space="preserve">
<value>Return type doesn't match overridden member because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInReturnTypeOnOverrideBecauseOfAttributes_Title" xml:space="preserve">
<value>Return type doesn't match overridden member because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnOverride" xml:space="preserve">
<value>Nullability of reference types in type of parameter '{0}' doesn't match overridden member.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnOverride_Title" xml:space="preserve">
<value>Nullability of reference types in type of parameter doesn't match overridden member.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnOverrideBecauseOfAttributes" xml:space="preserve">
<value>Type of parameter '{0}' doesn't match overridden member because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnOverrideBecauseOfAttributes_Title" xml:space="preserve">
<value>Type of parameter doesn't match overridden member because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnPartial" xml:space="preserve">
<value>Nullability of reference types in type of parameter '{0}' doesn't match partial method declaration.</value>
</data>
@@ -5369,6 +5381,18 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_NullabilityMismatchInParameterTypeOnImplicitImplementation_Title" xml:space="preserve">
<value>Nullability of reference types in type of parameter doesn't match implicitly implemented member.</value>
</data>
<data name="WRN_NullabilityMismatchInReturnTypeOnImplicitImplementationBecauseOfAttributes" xml:space="preserve">
<value>Nullability of reference types in return type of '{0}' doesn't match implicitly implemented member '{1}' because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInReturnTypeOnImplicitImplementationBecauseOfAttributes_Title" xml:space="preserve">
<value>Nullability of reference types in return type doesn't match implicitly implemented member because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnImplicitImplementationBecauseOfAttributes" xml:space="preserve">
<value>Nullability of reference types in type of parameter '{0}' of '{1}' doesn't match implicitly implemented member '{2}' because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnImplicitImplementationBecauseOfAttributes_Title" xml:space="preserve">
<value>Nullability of reference types in type of parameter doesn't match implicitly implemented member because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInTypeOnExplicitImplementation" xml:space="preserve">
<value>Nullability of reference types in type doesn't match implemented member '{0}'.</value>
</data>
@@ -5387,6 +5411,18 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_NullabilityMismatchInParameterTypeOnExplicitImplementation_Title" xml:space="preserve">
<value>Nullability of reference types in type of parameter doesn't match implemented member.</value>
</data>
<data name="WRN_NullabilityMismatchInReturnTypeOnExplicitImplementationBecauseOfAttributes" xml:space="preserve">
<value>Nullability of reference types in return type doesn't match implemented member '{0}' because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInReturnTypeOnExplicitImplementationBecauseOfAttributes_Title" xml:space="preserve">
<value>Nullability of reference types in return type doesn't match implemented member because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnExplicitImplementationBecauseOfAttributes" xml:space="preserve">
<value>Nullability of reference types in type of parameter '{0}' doesn't match implemented member '{1}' because of nullability attributes.</value>
</data>
<data name="WRN_NullabilityMismatchInParameterTypeOnExplicitImplementationBecauseOfAttributes_Title" xml:space="preserve">
<value>Nullability of reference types in type of parameter doesn't match implemented member because of nullability attributes.</value>
</data>
<data name="WRN_UninitializedNonNullableField" xml:space="preserve">
<value>Non-nullable {0} '{1}' is uninitialized. Consider declaring the {0} as nullable.</value>
</data>
@@ -5459,6 +5495,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_ShouldNotReturn_Title" xml:space="preserve">
<value>A method marked [DoesNotReturn] should not return.</value>
</data>
<data name="WRN_DoesNotReturnMismatch" xml:space="preserve">
<value>Method '{0}' lacks `[DoesNotReturn]` annotation to match implemented or overridden member.</value>
</data>
<data name="WRN_DoesNotReturnMismatch_Title" xml:space="preserve">
<value>Method lacks `[DoesNotReturn]` annotation to match implemented or overridden member.</value>
</data>
<data name="WRN_NullabilityMismatchInReturnTypeOfTargetDelegate" xml:space="preserve">
<value>Nullability of reference types in return type of '{0}' doesn't match the target delegate '{1}'.</value>
</data>
@@ -1746,6 +1746,16 @@ internal enum ErrorCode
WRN_ParameterConditionallyDisallowsNull = 8762,
WRN_ShouldNotReturn = 8763,

WRN_NullabilityMismatchInReturnTypeOnOverrideBecauseOfAttributes = 8764,
WRN_NullabilityMismatchInParameterTypeOnOverrideBecauseOfAttributes = 8765,

WRN_NullabilityMismatchInReturnTypeOnImplicitImplementationBecauseOfAttributes = 8766,
WRN_NullabilityMismatchInParameterTypeOnImplicitImplementationBecauseOfAttributes = 8767,
WRN_NullabilityMismatchInReturnTypeOnExplicitImplementationBecauseOfAttributes = 8768,
WRN_NullabilityMismatchInParameterTypeOnExplicitImplementationBecauseOfAttributes = 8769,

WRN_DoesNotReturnMismatch = 8770,

// Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)
}
}

0 comments on commit 961f261

Please sign in to comment.
You can’t perform that action at this time.