TODO removal

For each of these TODOs:

- If there's an active bug tracking the work, and the TODO provides
  something of value, I left it and standardized the formatting. I also
  added comments to the bug.

- If the comment provided no value (implement feature X when we do feature
  X), I deleted it with impunity.

- If the comment was stale (won't fix or just out of date), then we
  removed it uncerimoniously.

There was a single TODO that was actually actionable, so I enabled that
test.

samples/MvcSample.Web/FiltersController.cs

@@ -21,7 +21,6 @@ namespace MvcSample.Web
             CustomUser = new User() { Name = "User Name", Address = "Home Address" };
         }
 
-        // TODO: Add a real filter here
         [ServiceFilter(typeof(PassThroughAttribute))]
         [AllowAnonymous]
         [AgeEnhancer]

src/Microsoft.AspNet.Mvc.Core/AcceptVerbsAttribute.cs

@@ -32,8 +32,6 @@ namespace Microsoft.AspNet.Mvc
         /// <param name="methods">The HTTP methods the action supports.</param>
         public AcceptVerbsAttribute(params string[] methods)
         {
-            // TODO: This assumes that the methods are exactly same as standard Http Methods.
-            // The Http Abstractions should take care of these.
             _httpMethods = methods.Select(method => method.ToUpperInvariant());
         }
 

src/Microsoft.AspNet.Mvc.Core/AntiForgery/AntiForgeryOptions.cs

@@ -94,7 +94,6 @@ namespace Microsoft.AspNet.Mvc
             set;
         }
 
-        // TODO: Replace the stub.
         private string GetAntiForgeryCookieName()
         {
             return AntiForgeryTokenFieldName;

src/Microsoft.AspNet.Mvc.Core/AntiForgery/AntiForgeryWorker.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
 using System.Security.Claims;
 using System.Threading.Tasks;
 using Microsoft.AspNet.Http;
@@ -70,7 +69,6 @@ namespace Microsoft.AspNet.Mvc
                 if (user != null)
                 {
                     // We only support ClaimsIdentity.
-                    // Todo remove this once httpContext.User moves to ClaimsIdentity.
                     return user.Identity as ClaimsIdentity;
                 }
             }

src/Microsoft.AspNet.Mvc.Core/AntiForgery/DefaultClaimUidExtractor.cs

@@ -28,7 +28,6 @@ namespace Microsoft.AspNet.Mvc
 
         internal static IEnumerable<string> GetUniqueIdentifierParameters(ClaimsIdentity claimsIdentity)
         {
-            // TODO: Need to enable support for special casing acs identities.
             var nameIdentifierClaim = claimsIdentity.FindFirst(claim =>
                                                             String.Equals(ClaimTypes.NameIdentifier,
                                                                         claim.Type, StringComparison.Ordinal));

src/Microsoft.AspNet.Mvc.Core/Formatters/HttpNoContentOutputFormatter.cs

@@ -44,10 +44,6 @@ namespace Microsoft.AspNet.Mvc
         {
             var response = context.ActionContext.HttpContext.Response;
             response.ContentLength = 0;
-
-            // Only set the status code if its not already set.
-            // TODO: By default the status code is set to 200. 
-            // https://github.com/aspnet/HttpAbstractions/issues/114
             response.StatusCode = 204;
             return Task.FromResult(true);
         }

src/Microsoft.AspNet.Mvc.Core/Rendering/Html/TemplateRenderer.cs

@@ -94,7 +94,6 @@ namespace Microsoft.AspNet.Mvc.Rendering
                     using (var writer = new StringWriter(CultureInfo.InvariantCulture))
                     {
                         // Forcing synchronous behavior so users don't have to await templates.
-                        // TODO: Pass through TempData once implemented.
                         var view = viewEngineResult.View;
                         using (view as IDisposable)
                         {

src/Microsoft.AspNet.Mvc.HeaderValueAbstractions/MediaTypeHeaderValue.cs

@@ -76,7 +76,6 @@ namespace Microsoft.AspNet.Mvc.HeaderValueAbstractions
                 return false;
             }
 
-            // TODO: throw if the media type and subtypes are invalid.
             var mediaType = mediaTypeParts[0].Trim();
             var mediaSubType = mediaTypeParts[1].Trim();
             var mediaTypeRange = MediaTypeHeaderValueRange.None;
@@ -118,7 +117,6 @@ namespace Microsoft.AspNet.Mvc.HeaderValueAbstractions
                 var index = parameter.Split('=');
                 if (index.Length == 2)
                 {
-                    // TODO: throw exception if this is not the case.
                     parameterNameValue.Add(index[0].Trim(), index[1].Trim());
                 }
             }

src/Microsoft.AspNet.Mvc.HeaderValueAbstractions/StringWithQualityHeaderValue.cs

@@ -27,7 +27,6 @@ namespace Microsoft.AspNet.Mvc.HeaderValueAbstractions
                 var nameValuePair = parameter.Split(new[] { '=' }, 2);
                 if (nameValuePair.Length > 1 && nameValuePair[0].Trim().Equals("q"))
                 {
-                    // TODO: all extraneous parameters are ignored. Throw/return null if that is the case.
                     if (!double.TryParse(
                             nameValuePair[1],
                             NumberStyles.AllowLeadingWhite | NumberStyles.AllowDecimalPoint |

src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Threading.Tasks;
 
 namespace Microsoft.AspNet.Mvc.ModelBinding
@@ -95,9 +96,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
         private async Task<bool> TryBind(ModelBindingContext bindingContext)
         {
-            // TODO: RuntimeHelpers.EnsureSufficientExecutionStack does not exist in the CoreCLR.
-            // Protects against stack overflow for deeply nested model binding
-            // RuntimeHelpers.EnsureSufficientExecutionStack();
+            RuntimeHelpers.EnsureSufficientExecutionStack();
 
             foreach (var binder in ModelBinders)
             {

src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs

@@ -141,8 +141,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
                 if (validationState == ModelValidationState.Unvalidated)
                 {
-                    // Tracked via https://github.com/aspnet/Mvc/issues/450
-                    // TODO: Revive ModelBinderConfig
+                    // TODO: https://github.com/aspnet/Mvc/issues/450 Revive ModelBinderConfig
                     // var errorMessage =  ModelBinderConfig.ValueRequiredErrorMessageProvider(e.ValidationContext, 
                     //                                                                            modelMetadata, 
                     //                                                                            incomingValue);

src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeMatchModelBinder.cs

@@ -22,7 +22,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref model);
             bindingContext.Model = model;
 
-            // TODO: Determine if we need IBodyValidator here.
             return true;
         }
 

src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/FormValueProviderFactory.cs

@@ -37,8 +37,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
         private static CultureInfo GetCultureInfo(HttpRequest request)
         {
-            // TODO: Tracked via https://github.com/aspnet/HttpAbstractions/issues/10. 
-            // Determine what's the right way to map Accept-Language to culture.
             return CultureInfo.CurrentCulture;
         }
     }

src/Microsoft.AspNet.Mvc.Razor.Host/Directives/ChunkInheritanceUtility.cs

@@ -101,8 +101,6 @@ namespace Microsoft.AspNet.Mvc.Razor.Directives
             {
                 if (mergerMappings.TryGetValue(chunk.GetType(), out merger))
                 {
-                    // TODO: When mapping chunks, we should remove mapping information since it would be incorrect
-                    // to generate it in the page that inherits it. Tracked by #945
                     merger.Merge(codeTree, chunk);
                 }
             }
@@ -119,7 +117,6 @@ namespace Microsoft.AspNet.Mvc.Razor.Directives
             };
         }
 
-        // TODO: This needs to be cached (#1016)
         private static CodeTree ParseViewFile(RazorTemplateEngine engine,
                                               IFileInfo fileInfo)
         {

src/Microsoft.AspNet.Mvc.Razor.Host/MvcCSharpCodeBuilder.cs

@@ -67,9 +67,6 @@ namespace Microsoft.AspNet.Mvc.Razor
 
         protected override void BuildConstructor([NotNull] CSharpCodeWriter writer)
         {
-            // TODO: Move this to a proper extension point. Right now, we don't have a place to print out properties
-            // in the generated view.
-            // Tracked by #773
             base.BuildConstructor(writer);
 
             writer.WriteLineHiddenDirective();

src/Microsoft.AspNet.Mvc.Razor/Razor/PreCompileViews/RazorFileInfoCollectionGenerator.cs

@@ -35,7 +35,6 @@ namespace Microsoft.AspNet.Mvc.Razor
 
             builder.Append(Bottom);
 
-            // TODO: consider saving the file for debuggability
             var sourceCode = builder.ToString();
             var syntaxTree = SyntaxTreeGenerator.Generate(sourceCode, 
                                                           "__AUTO__GeneratedViewsCollection.cs",

src/Microsoft.AspNet.Mvc.TagHelpers/AnchorTagHelper.cs

@@ -135,7 +135,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             }
         }
 
-        // TODO: We will not need this method once https://github.com/aspnet/Razor/issues/89 is completed.
+        // TODO: https://github.com/aspnet/Razor/issues/89 - We will not need this method once #89 is completed.
         private static Dictionary<string, object> GetRouteValues(
             TagHelperOutput output, IEnumerable<KeyValuePair<string, string>> routePrefixedAttributes)
         {

src/Microsoft.AspNet.Mvc.TagHelpers/FormTagHelper.cs

@@ -113,7 +113,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             }
         }
 
-        // TODO: We will not need this method once https://github.com/aspnet/Razor/issues/89 is completed.
+        // TODO: https://github.com/aspnet/Razor/issues/89 - We will not need this method once #89 is completed.
         private static Dictionary<string, object> GetRouteValues(
             TagHelperOutput output, IEnumerable<KeyValuePair<string, string>> routePrefixedAttributes)
         {

src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperOutputExtensions.cs

@@ -50,7 +50,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
         public static IEnumerable<KeyValuePair<string, string>> FindPrefixedAttributes(
             this TagHelperOutput tagHelperOutput, string prefix)
         {
-            // TODO: We will not need this method once https://github.com/aspnet/Razor/issues/89 is completed.
+            // TODO: https://github.com/aspnet/Razor/issues/89 - We will not need this method once #89 is completed.
 
             // We're only interested in HTML attributes that have the desired prefix.
             var prefixedAttributes = tagHelperOutput.Attributes

src/Microsoft.AspNet.Mvc.TagHelpers/ValidationSummaryTagHelper.cs

@@ -24,7 +24,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
         [Activate]
         protected internal IHtmlGenerator Generator { get; set; }
 
-        // TODO: Change to ValidationSummary enum once https://github.com/aspnet/Razor/issues/196 has been completed.
+        // TODO: https://github.com/aspnet/Razor/issues/196 Change to ValidationSummary enum once #196 has been completed.
         /// <summary>
         /// If <c>All</c> or <c>ModelOnly</c>, appends a validation summary. Acceptable values are defined by the 
         /// <see cref="ValidationSummary"/> enum.

test/Microsoft.AspNet.Mvc.Core.Test/AntiXsrf/AntiForgeryTokenStoreTest.cs

@@ -269,7 +269,6 @@ namespace Microsoft.AspNet.Mvc.Core.Test
             var token = new AntiForgeryToken();
             var mockCookies = new Mock<IResponseCookies>();
 
-            // TODO : Once we decide on where to pick this value from enable this.
             bool defaultCookieSecureValue = expectedCookieSecureFlag ?? false; // pulled from config; set by ctor
             var cookies = new MockResponseCookieCollection();
 

test/Microsoft.AspNet.Mvc.Core.Test/Rendering/HtmlHelperNameExtensionsTest.cs

@@ -12,10 +12,6 @@ namespace Microsoft.AspNet.Mvc.Core
     /// <summary>
     /// Test the <see cref="HtmlHelperNameExtensions" /> class.
     /// </summary>
-    /// <remarks>
-    /// TODO #704: When that bug is fixed and Id() behaves differently than Name(), will need to break some
-    /// test methods below in two.
-    /// </remarks>
     public class HtmlHelperNameExtensionsTest
     {
         [Fact]

test/Microsoft.AspNet.Mvc.FunctionalTests/RoundTripTests.cs

@@ -105,7 +105,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
 
             // Assert
             Assert.Equal("Parent.Attributes[height]", expression);
-            // TODO Requires resolution in model binding as part of #1418
+            // TODO: https://github.com/aspnet/Mvc/issues/1418 Requires resolution in model binding
         }
 
         // Uses the expression p => p.Dependents[0].Dependents[0].Name

test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/AssociatedMetadataProviderTest.cs

@@ -201,8 +201,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
         // Helpers
 
-        // TODO: This type used System.ComponentModel.MetadataType to separate attribute declaration from property
-        // declaration. Need to figure out if this is still relevant since the type does not exist in CoreCLR.
         private class PropertyModel
         {
             [Required]

test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/CachedDataAnnotationsModelMetadataProviderTest.cs

@@ -167,7 +167,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             Assert.True(result);
         }
 
-        // TODO #1000; enable test once we detect attributes on the property's type
+        // TODO https://github.com/aspnet/Mvc/issues/1000
+        // Enable test once we detect attributes on the property's type
         public void HiddenInputWorksOnPropertyType()
         {
             // Arrange

test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/ReadableStringCollectionValueProviderTests.cs

@@ -157,8 +157,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
             Assert.Equal(culture, vpResult.Culture);
         }
 
-        // TODO: Determine if this is still relevant. Right now the lookup returns null while
-        // we expect a ValueProviderResult that wraps a null value.
+        // TODO: https://github.com/aspnet/Mvc/issues/1609
+        //
+        // This test was present in MVC5 and WebAPI2, but our VP implementation doesn't behave this
+        // way currently.
+        //
         //[Theory]
         //[InlineData("null_value")]
         //[InlineData("prefix.null_value")]
@@ -166,16 +169,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
         //{
         //    // Arrange
         //    var culture = new CultureInfo("fr-FR");
-        //    var valueProvider = new ReadableStringCollectionValueProvider(_backingStore, culture);
+        //    var valueProvider = new ReadableStringCollectionValueProvider<TestValueProviderMetadata>(_backingStore, culture);
+
+        //    System.Diagnostics.Debugger.Launch();
+        //    System.Diagnostics.Debugger.Break();
 
         //    // Act
-        //    ValueProviderResult vpResult = valueProvider.GetValue(key);
+        //    var result = await valueProvider.GetValueAsync(key);
 
         //    // Assert
-        //    Assert.NotNull(vpResult);
-        //    Assert.Equal(null, vpResult.RawValue);
-        //    Assert.Equal(null, vpResult.AttemptedValue);
-        //    Assert.Equal(culture, vpResult.Culture);
+        //    Assert.NotNull(result);
+        //    Assert.Null(result.RawValue);
+        //    Assert.Null(result.AttemptedValue);
+        //    Assert.Equal(culture, result.Culture);
         //}
 
         [Fact]

test/Microsoft.AspNet.Mvc.TagHelpers.Test/InputTagHelperTest.cs

@@ -57,8 +57,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
                     { modelWithText, typeof(NestedModel), () => modelWithText.NestedModel.Text,
                         new NameAndId("NestedModel.Text", "NestedModel_Text"), "inner text" },
 
-                    // Top-level indexing does not work end-to-end due to code generation issue #1345.
-                    // TODO: Remove above comment when #1345 is fixed.
                     { models, typeof(Model), () => models[0].Text,
                         new NameAndId("[0].Text", "z0__Text"), string.Empty },
                     { models, typeof(Model), () => models[1].Text,

test/Microsoft.AspNet.Mvc.TagHelpers.Test/SelectTagHelperTest.cs

@@ -70,13 +70,12 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
                     { modelWithText, typeof(NestedModel), () => modelWithText.NestedModel.Text,
                         new NameAndId("NestedModel.Text", "NestedModel_Text"), innerSelected },
 
-                    // Top-level indexing does not work end-to-end due to code generation issue #1345.
-                    // TODO: Remove above comment when #1345 is fixed.
                     { models, typeof(Model), () => models[0].Text,
                         new NameAndId("[0].Text", "z0__Text"), noneSelected },
                     { models, typeof(NestedModel), () => models[0].NestedModel.Text,
                         new NameAndId("[0].NestedModel.Text", "z0__NestedModel_Text"), noneSelected },
 
+                    // TODO: https://github.com/aspnet/Mvc/issues/1468
                     // Skip last two test cases because DefaultHtmlGenerator evaluates expression name against
                     // ViewData, not using ModelMetadata.Model. ViewData.Eval() handles simple property paths and some
                     // dictionary lookups, but not indexing into an array or list. See #1468...
@@ -316,9 +315,10 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             var tagHelperContext = new TagHelperContext(contextAttributes);
             var output = new TagHelperOutput(expectedTagName, originalAttributes, content);
 
-            // TODO: In real (model => model) scenario, ModelExpression should have name "" and
+            // TODO: https://github.com/aspnet/Mvc/issues/1253
+            // In real (model => model) scenario, ModelExpression should have name "" and
             // TemplateInfo.HtmlFieldPrefix should be "Property1" but empty ModelExpression name is not currently
-            // supported, see #1408.
+            // supported, see also #1408.
             var metadataProvider = new EmptyModelMetadataProvider();
             string model = null;
             var metadata = metadataProvider.GetMetadataForType(() => model, typeof(string));

test/Microsoft.AspNet.Mvc.TagHelpers.Test/TextAreaTagHelperTest.cs

@@ -61,8 +61,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
                         new NameAndId("NestedModel.Text", "NestedModel_Text"),
                         Environment.NewLine + "inner text" },
 
-                    // Top-level indexing does not work end-to-end due to code generation issue #1345.
-                    // TODO: Remove above comment when #1345 is fixed.
                     { models, typeof(Model), () => models[0].Text,
                         new NameAndId("[0].Text", "z0__Text"),
                         Environment.NewLine },

test/WebSites/BasicWebSite/Controllers/HomeController.cs

@@ -50,9 +50,6 @@ namespace BasicWebSite.Controllers
 
         public async Task ActionReturningTask()
         {
-            // TODO: #1077. With HttpResponseMessage, there seems to be a race between the write operation setting the
-            // header to 200 and HttpNoContentOutputFormatter which sets the header to 204.
-            Context.Response.StatusCode = 204;
             await Context.Response.WriteAsync("Hello world");
         }