Add a DeclareAsNullable fixer #26630

jcouv · 2018-05-04T18:29:04Z

This fixer is designed to automatically fix only some trivial cases (so that FixAll is appropriate):

local declaration: string x = null; (note: multi-declarations don't get fixed)
return type: string M() { return null } and string M() => null;
optional parameters: void M(string x = null) ... (depends on a compiler fix for null default value for non-nullable parameter should warn #26626)
properties: string x { get; set; } = null; (depends on a compiler fix for null initializer for non-nullable property (or field) should warn #26628), string x => null;

This is to support customers migrating their projects to use nullable annotations, including our own dogfooding.

@dotnet/roslyn-ide for review.
@dotnet/roslyn-compiler FYI.

sharwell · 2018-05-04T18:39:07Z

local declaration

This is the one I'm most apprehensive about. When you compare the compiler code updates to the IDE code updates required for nullable-aware code, #23281 becomes painfully apparent.

Neme12 · 2018-05-04T18:50:59Z

src/EditorFeatures/CSharpTest/Diagnostics/Nullable/DeclareAsNullableCodeFixTests.cs

+namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics.DeclareAsNullable
+{
+    [Trait(Traits.Feature, Traits.Features.CodeActionsDeclareAsNullable)]
+    public partial class DeclareAsNullableCodeFixTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest


why partial? #Resolved

Neme12 · 2018-05-04T18:53:28Z

src/EditorFeatures/CSharpTest/Diagnostics/Nullable/DeclareAsNullableCodeFixTests.cs

+
+        [Fact]
+        [WorkItem(26628, "https://github.com/dotnet/roslyn/issues/26628")]
+        public async Task FixPropertyDeclaration()


I think a better approach might be to change the test to the actual desired behavior and add Skip with issue link. I think WorkItem is used for bugs that have been fixed (not for things that still need attention), isn't that the case? #Resolved

I'll change, but WorkItem is fine. What matters is there is a tracking issue to drive fixing the problem :-) #Resolved

Neme12 · 2018-05-04T18:54:13Z

src/EditorFeatures/CSharpTest/Diagnostics/Nullable/DeclareAsNullableCodeFixTests.cs

+{
+    static void M(string x = [|null|]) { }
+}", parameters: s_nullableFeature);
+        }


consider adding a test with var #Resolved

consider adding a test with a field #Resolved

Neme12 · 2018-05-04T18:57:55Z

src/Features/CSharp/Portable/CSharpFeaturesResources.resx

@@ -162,6 +162,9 @@
  <data name="Remove_Unnecessary_Cast" xml:space="preserve">
    <value>Remove Unnecessary Cast</value>
  </data>
+  <data name="Declare_as_Nullable" xml:space="preserve">
+    <value>Declare as Nullable</value>


unrelated: I see the casing is quite inconsistent between features. For example "Remove Unnecessary Cast" above and "Cast is redundant" below. #Resolved

I changed to lower case. I think it looks nicer, and the other strings below give me an excuse ;-)

In reply to: 186184879 [](ancestors = 186184879)

Neme12 · 2018-05-04T18:58:30Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+namespace Microsoft.CodeAnalysis.CSharp.CodeFixes.DeclareAsNullable
+{
+    [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.DeclareAsNullable), Shared]
+    internal partial class DeclareAsNullableCodeFixProvider : SyntaxEditorBasedCodeFixProvider


why partial? #Resolved

nit: looking at the names of other code fixes, should this be called CSharpDeclareAsNullableCodeFixProvider? #Resolved

Neme12 · 2018-05-04T19:02:38Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+
+            foreach (var diagnostic in diagnostics)
+            {
+                var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);


~~is it possible that after 1 fix is applied, this location for another diagnostic would be outdated and therefore we might not find the correct node?~~ sorry, I see we're looking at the original root #Resolved

Neme12 · 2018-05-04T19:04:43Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+
+        private static TypeSyntax TryGetDeclarationTypeToFix(SyntaxNode node)
+        {
+            if (!node.IsKind(SyntaxKind.NullLiteralExpression))


should this be supported for default literals as well? or any expression that has a constant value of null if that's supported by the compiler? #Resolved

I'm going to reserve judgement on that one for now. I want to address the cases that are very clear first, and gain experience from dogfooding. We can add later if valuable.

In reply to: 186186557 [](ancestors = 186186557)

Neme12 · 2018-05-04T19:05:30Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+            // return null;
+            if (node.IsParentKind(SyntaxKind.ReturnStatement))
+            {
+                var methodDeclaration = node.GetAncestors<MethodDeclarationSyntax>().FirstOrDefault();


this needs to account for local functions as well #Resolved

Added a skipped test and filed a compiler bug for local functions.

In reply to: 186186732 [](ancestors = 186186732)

Neme12 · 2018-05-04T19:07:34Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+            if (node.Parent?.Parent?.IsParentKind(SyntaxKind.VariableDeclaration) == true)
+            {
+                var variableDeclaration = (VariableDeclarationSyntax)node.Parent.Parent.Parent;
+                if (variableDeclaration.Variables.Count != 1)


~~note: there are probably other cases that need to be supported, such as a for each statement variable. if you don't implement this, please create an issue so that somebody can fix this later~~ I'm probably wrong. I don't see how this could come up in a for each statement. Are we sure there are no other cases though? #Resolved

I can't think of any other scenario at the moment. But we can always fill any gaps we discover. I'll be testing this on Roslyn codebase, so that may give some ideas. #Resolved

Neme12 · 2018-05-04T19:08:51Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+            if (node.Parent?.IsParentKind(SyntaxKind.PropertyDeclaration) == true)
+            {
+                var propertyDeclaration = (PropertyDeclarationSyntax)node.Parent.Parent;
+                return propertyDeclaration.Type;


we might also want to offer this inside a property getter or setter:

string X { get { return null; } } ``` #Resolved

Neme12 · 2018-05-04T19:09:24Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+            // static string M() => null;
+            if (node.IsParentKind(SyntaxKind.ArrowExpressionClause) && node.Parent.IsParentKind(SyntaxKind.MethodDeclaration))
+            {
+                var arrowMethod = (MethodDeclarationSyntax)node.Parent.Parent;


same comment about local functions and maybe properties #Resolved

Neme12 · 2018-05-04T19:10:47Z

src/Features/CSharp/Portable/CodeFixes/Nullable/DeclareAsNullableCodeFixProvider.cs

+            return Task.CompletedTask;
+        }
+
+        internal static void MakeDeclarationNullable(Document document, SyntaxEditor editor, SyntaxNode node)


I think this doesn't need to be internal? #Resolved

jcouv · 2018-05-04T19:38:29Z

📝 I'm investigating a bug with FixAll on a method with multiple return null; statements. The declaration should only be fixed once ;-) #Resolved

CyrusNajmabadi · 2018-05-04T22:19:46Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+            // warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter.
+            // warning CS8600: Converting null literal or possible null value to non-nullable type.
+            get { return ImmutableArray.Create("CS8625", "CS8600"); }
+        }


can just be FixableDiagnosticIds { get; } = or =>

#Resolved

CyrusNajmabadi · 2018-05-04T22:20:23Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+
+            foreach (var diagnostic in diagnostics)
+            {
+                var node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);


diagnostic.Location.FindNode(...) #Resolved

Neme12 · 2018-05-04T22:24:45Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

            }

            return Task.CompletedTask;
        }

-        internal static void MakeDeclarationNullable(Document document, SyntaxEditor editor, SyntaxNode node)
+        private void MakeDeclarationNullable(Document document, SyntaxEditor editor, SyntaxNode node, HashSet<TypeSyntax> alreadyHandled)


please keep static #Resolved

I had the hashset as instance field for a while. I'll restore static. #Resolved

Neme12 · 2018-05-04T22:25:36Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

nit: extra blank line #Resolved

CyrusNajmabadi

changes

CyrusNajmabadi · 2018-05-04T22:21:21Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+                var fixedDeclaration = SyntaxFactory.NullableType(declarationTypeToFix);
+                editor.ReplaceNode(declarationTypeToFix, fixedDeclaration);
+                alreadyHandled.Add(declarationTypeToFix);
+            }


this will probably do the wrong with wrt trivia. we should do SyntaxFactory.NullableType(declarationTypeToFix.WithoutTrivia()).WithTriviaFrom(declarationType); #Resolved

can just do . && alreadyHandled.Add(declarationType) #Resolved

Can you have trivia tests? #Resolved

CyrusNajmabadi · 2018-05-04T22:21:52Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+            var root = editor.OriginalRoot;
+
+            // a method can have multiple `return null;` statements, but we should only fix its return type once
+            var alreadyHandled = new HashSet<TypeSyntax>();


consider pooling this. though ti's just one doc, so likely not too bad. #Resolved

CyrusNajmabadi · 2018-05-04T22:22:10Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+namespace Microsoft.CodeAnalysis.CSharp.CodeFixes.DeclareAsNullable
+{
+    [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.DeclareAsNullable), Shared]
+    internal class CSharpDeclareAsNullableCodeFixProvider : SyntaxEditorBasedCodeFixProvider


What's the status on VB and nullable? happening or not? #Resolved

Haven't heard any update. We're trying to wrap up nullable for C# first.

CyrusNajmabadi · 2018-05-04T22:22:48Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+
+            if (node.IsParentKind(SyntaxKind.ReturnStatement))
+            {
+                var containingMember = node.GetAncestors().FirstOrDefault(a => a.IsKind(SyntaxKind.MethodDeclaration, SyntaxKind.PropertyDeclaration));


what if the containing member is a local function? #Resolved

It'll be ignored for now. There is a tracking bug.

In reply to: 186234771 [](ancestors = 186234771)

Note: i would not add skipped tests for this. Just add the tests with the current bahvior and the note that this will change when the corresponding bug is fixed. That way they do break and are appropriately updated when the change is made . Otherwise, they can just sit there skipped with people not noticing for years. #Resolved

@CyrusNajmabadi I'm sorry, that is kind of my fault if you look at the feedback above 😄 #Resolved

@jcouv I'm sorry to bother you about that :-/ #Resolved

My reasoning was that regular tests with WorkAttribute would go unnoticed without raising any eyebrows as opposed to skipped tests. Sorry again you had to change this twice. #Resolved

No worries. Both rationales offer some benefits. Cyrus's argument (easier to forget skipped tests) carried the day ;-) #Resolved

CyrusNajmabadi · 2018-05-04T22:23:40Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+            }
+
+            // void M(string x = null) { }
+            if (node.Parent?.IsParentKind(SyntaxKind.Parameter) == true)


no need for ? before IsParentKind, the Is helpers do the right thing on null (they are trying to emulate the 'is' language feature). #Resolved

CyrusNajmabadi · 2018-05-04T22:24:15Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+            }
+
+            // static string M() => null;
+            if (node.IsParentKind(SyntaxKind.ArrowExpressionClause) && node.Parent.IsParentKind(SyntaxKind.MethodDeclaration))


what about local functions? #Resolved

@CyrusNajmabadi I have asked about that before. @jcouv created an issue to track this: #26639 #Resolved

jcouv · 2018-05-04T22:48:41Z

The purpose for this fixer was just to handle the trivial cases, that are not controversial, and can be used as FixAll.

I'll start a second fixer that will be meant for individual fixes (no FixAll). For instance, you have something == null or something != null in your code, that is a strong indication that something should be declared as nullable. But it's not 100% guaranteed (you could be programming defensively). In such case, fix the declaration of something with just one press of Ctrl+..

Beyond that, I'm open to suggestions, but I expect the dogfooding experience will give more ideas ;-)

jcouv · 2018-05-04T22:55:26Z

@CyrusNajmabadi 304 changes is only for those trivially recognizable warnings in the CodeAnalysis project alone (I didn't look at CSharpCodeAnalysis or IDE code). There are ~1500 warnings left to fix beyond that, in CodeAnalysis ;-) #Resolved

CyrusNajmabadi · 2018-05-04T22:57:29Z

in the CodeAnalysis project

Ohhhh, that makes a lot more sense. #Resolved

jcouv · 2018-05-07T18:29:21Z

@dotnet/roslyn-ide We'd like to include this in the prototype we'll share at BUILD tomorrow. The feedback from Cyrus and Neme has been addressed. Please review. #Resolved

sharwell · 2018-05-07T19:11:31Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+
+            if (node.IsParentKind(SyntaxKind.ReturnStatement))
+            {
+                var containingMember = node.GetAncestors().FirstOrDefault(a => a.IsKind(SyntaxKind.MethodDeclaration, SyntaxKind.PropertyDeclaration));


❗️ It's fine to not support anonymous/local functions, but you need to at least detect the case and not try to modify the return value of an unrelated method. Since this is intended for Fix All scenarios, users will end up changing values that don't need to change and in the transitive closure it will be a mess. The resolution is to add local functions and the various lambda syntaxes to this upward search, and for anything not supported return without making changes. #Resolved

The compiler doesn't produce the triggering warnings in such cases. But I do expect it will (soon) in the case of local functions. I have a test that guards that, and also added defensive code just in case.
#Resolved

sharwell · 2018-05-07T19:54:35Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+            if (node.IsParentKind(SyntaxKind.ReturnStatement))
+            {
+                var containingMember = node.GetAncestors().FirstOrDefault(a => a.IsKind(SyntaxKind.MethodDeclaration, SyntaxKind.PropertyDeclaration,
+                    SyntaxKind.ParenthesizedLambdaExpression, SyntaxKind.SimpleLambdaExpression, SyntaxKind.LocalFunctionStatement));


❕ Also need AnonymousMethodExpression #Resolved

Good catch. Thanks

I think you could use FirstOrDefault(a => a.IsReturnableConstruct()) (in SyntaxNodeExtensions)

also, isn't there a method like GetAncestorOrThis or FirstAncestorOrSelf that takes a predicate somewhere?

IsReturnableConstruct

Even better. Thanks!

isn't there a method like ... that takes a predicate somewhere?

Didn't find one. I'll leave as-is.

Actually... that might be wrong. IsReturnableConstruct considers individual accessors whereas you're looking at the actual property. I don't think this will work. Sorry :/

sharwell · 2018-05-07T19:55:02Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+                        // string x { get { return null; } }
+                        return ((PropertyDeclarationSyntax)containingMember).Type;
+
+                    case SyntaxKind.ParenthesizedLambdaExpression:


💡 May as well use default: #Resolved

mavasani · 2018-05-07T20:16:14Z

This fixer is designed to automatically fix only some trivial cases

@jcouv Do we have a tracking issue to handle the remaining case?

sharwell · 2018-05-07T20:20:34Z

Do we have a tracking issue to handle the remaining case?

It's a fairly fluid situation at this point. The common cases are being handled as they are observed at scale, and we can think about how to handle the remaining cases once the vision for the underlying feature becomes clearer.

The New Language Feature - Nullable Reference Types label is probably the best way to track related issues.

CyrusNajmabadi · 2018-05-07T20:28:44Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

@@ -80,8 +80,7 @@ private static TypeSyntax TryGetDeclarationTypeToFix(SyntaxNode node)

            if (node.IsParentKind(SyntaxKind.ReturnStatement))
            {
-                var containingMember = node.GetAncestors().FirstOrDefault(a => a.IsKind(SyntaxKind.MethodDeclaration, SyntaxKind.PropertyDeclaration,
-                    SyntaxKind.ParenthesizedLambdaExpression, SyntaxKind.SimpleLambdaExpression, SyntaxKind.LocalFunctionStatement, SyntaxKind.AnonymousMethodExpression));
+                var containingMember = node.GetAncestors().FirstOrDefault(a => a.IsReturnableConstruct());


Does that work properly for accessors/properties?

Hum, good catch. A test did regress. I'll adjust.

jcouv · 2018-05-07T20:31:32Z

In addition to what Sam said, we're also tracking many work items in the nullable umbrella issue.

As far as how to improve the nullable fixer, I'm dogfooding the nullability preview on Roslyn, and using that experience to give me ideas of what can be automated.
So far, the second set of scenarios I've identified as somewhat automatable (although probably not to the confidence level of a FixAll) is covered in PR #26661 (still WIP).

Neme12 · 2018-05-07T20:36:10Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+                        return ((PropertyDeclarationSyntax)containingMember).Type;
+
+                    default:
+                        return null;


you could go back and remove IsReturnableConstruct or I would persnally keep it and use pattern matching here to get the property from the accessor we found:

switch (containingMember) { case MethodDeclarationSyntax method: ... case AccessorDeclarationSyntax accessor when accessor.Parent is PropertyDeclarationSyntax property: ... }

this makes me think, what about indexers? they should probably be handled too right?

jcouv · 2018-05-07T20:51:40Z

src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs

@@ -100,6 +100,17 @@ public static bool IsKind(this SyntaxNode node, SyntaxKind kind1, SyntaxKind kin
            return csharpKind == kind1 || csharpKind == kind2 || csharpKind == kind3 || csharpKind == kind4 || csharpKind == kind5 || csharpKind == kind6;
        }

+        public static bool IsKind(this SyntaxNode node, SyntaxKind kind1, SyntaxKind kind2, SyntaxKind kind3, SyntaxKind kind4, SyntaxKind kind5, SyntaxKind kind6, SyntaxKind kind7, SyntaxKind kind8, SyntaxKind kind9, SyntaxKind kind10, SyntaxKind kind11)


Surely, nobody will ever want more than 11, right? ;-)

Neme12 · 2018-05-07T20:52:06Z

src/Features/CSharp/Portable/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

+                var containingMember = node.GetAncestors().FirstOrDefault(a => a.IsKind(
+                    SyntaxKind.MethodDeclaration, SyntaxKind.PropertyDeclaration, SyntaxKind.ParenthesizedLambdaExpression, SyntaxKind.SimpleLambdaExpression,
+                    SyntaxKind.LocalFunctionStatement, SyntaxKind.AnonymousMethodExpression, SyntaxKind.ConstructorDeclaration, SyntaxKind.DestructorDeclaration,
+                    SyntaxKind.OperatorDeclaration, SyntaxKind.IndexerDeclaration, SyntaxKind.EventDeclaration));

                if (containingMember == null)


nit: this is now unnecessary with pattern matching

I don't want to use pattern matching here because it involves many type tests, repeated for each level of ancestry.

I meant the pattern matching that you're already using below. Not that you should use it above as well.

it involves many type tests, repeated for each level of ancestry

Is there a performance problem with that? I've seen GetAncestorOrSelf<T> and such used all over the place.

Oh, right. That one (below) is fine :-)

Yes, I suspect there would be some performance impact. My prior could be changed with some micro-benchmark, once someone takes the time to check.

The problem is not with GetAncestor method itself. But it's a loop, and you want to make iterations cheaper when in such a tight loop.

Neme12 · 2018-05-07T20:54:33Z

src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs

+            }
+
+            var csharpKind = node.Kind();
+            return csharpKind == kind1 || csharpKind == kind2 || csharpKind == kind3 || csharpKind == kind4 || csharpKind == kind5 || csharpKind == kind6 || csharpKind == kind7 || csharpKind == kind8 || csharpKind == kind9 || csharpKind == kind10 || csharpKind == kind11;


nit: consider wrapping

jcouv added 4 commits May 4, 2018 10:20

Add DeclareAsNullable fixer, handling return null

a08f897

Handle local declaration initialized with null

5fe73bd

Handle optional parameters and properties

c03da9f

Handle method with arrow body

4d4b823

jcouv added Area-IDE Feature - Nullable Reference Types Nullable Reference Types labels May 4, 2018

jcouv added this to the 16.0 milestone May 4, 2018

jcouv self-assigned this May 4, 2018

jcouv requested a review from a team as a code owner May 4, 2018 18:29

Neme12 reviewed May 4, 2018

View reviewed changes

CyrusNajmabadi reviewed May 4, 2018

View reviewed changes

FixAll doesn't repeat work. Address PR feedback from Neme

ce7cb82

jcouv force-pushed the nullable-fixer branch from bf25a9e to ce7cb82 Compare May 4, 2018 22:20

Neme12 reviewed May 4, 2018

View reviewed changes

CyrusNajmabadi requested changes May 4, 2018

View reviewed changes

Address PR feedback from Cyrus

6285c85

jcouv mentioned this pull request May 6, 2018

[Work items] Nullable reference types #22152

Closed

jcouv added the Urgency-Soon label May 7, 2018

sharwell requested changes May 7, 2018

View reviewed changes

Protect again lambda or local function cases

2c8b4f2

jcouv force-pushed the nullable-fixer branch from 0184bbf to 2c8b4f2 Compare May 7, 2018 19:53

sharwell approved these changes May 7, 2018

View reviewed changes

Check for anonymous method expression too

b3d1218

jcouv added the Auto-Merge If Tests Pass label May 7, 2018

mavasani approved these changes May 7, 2018

View reviewed changes

jcouv removed the Auto-Merge If Tests Pass label May 7, 2018

Use IsReturnableConstruct

dbe008b

CyrusNajmabadi reviewed May 7, 2018

View reviewed changes

Neme12 reviewed May 7, 2018

View reviewed changes

Revert usage of IsReturnableConstruct

83c8615

jcouv commented May 7, 2018

View reviewed changes

Neme12 reviewed May 7, 2018

View reviewed changes

jcouv mentioned this pull request May 7, 2018

Nullable reference types bias towards use of 'var' #23281

Closed

jcouv merged commit 5902f69 into dotnet:features/NullableReferenceTypes May 7, 2018

jcouv deleted the nullable-fixer branch May 7, 2018 22:52

jcouv mentioned this pull request Sep 23, 2018

DeclareAsNullable fixer should trigger when assigning "maybe null" values to non-null variable #30110

Closed

jcouv removed the Urgency-Soon label Oct 12, 2018

Add a DeclareAsNullable fixer #26630

Add a DeclareAsNullable fixer #26630

Conversation

jcouv commented May 4, 2018

sharwell commented May 4, 2018

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv commented May 4, 2018 • edited Loading

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

Neme12 May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv May 4, 2018 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi May 4, 2018 • edited by jcouv Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

jcouv May 4, 2018 •

edited

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

jcouv May 4, 2018 •

edited

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

jcouv commented May 4, 2018 •

edited

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

jcouv May 4, 2018 •

edited

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

jcouv May 4, 2018 •

edited

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

CyrusNajmabadi May 4, 2018 •

edited by jcouv

Loading

Neme12 May 4, 2018 •

edited by jcouv

Loading

jcouv commented May 4, 2018 •

edited

Loading

CyrusNajmabadi commented May 4, 2018 •

edited by jcouv

Loading

jcouv commented May 7, 2018 •

edited

Loading

sharwell May 7, 2018 •

edited by jcouv

Loading

jcouv May 7, 2018 •

edited

Loading

sharwell May 7, 2018 •

edited by jcouv

Loading

Neme12 May 7, 2018 •

edited

Loading

Neme12 May 7, 2018 •

edited

Loading

sharwell May 7, 2018 •

edited by jcouv

Loading

sharwell commented May 7, 2018 •

edited

Loading

Neme12 May 7, 2018 •

edited

Loading

Neme12 May 7, 2018 •

edited

Loading

jcouv May 7, 2018 •

edited

Loading