dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.29k stars 4.74k forks source link

DateTime does not contain a definition for 'ToShortDateString' #4783

Closed adun closed 4 years ago

adun commented 8 years ago

Description

I'm facing an issue using System.DateTime class. I've got the following error:

DNXCore,Version=v5.0 error CS1061: 'DateTime' does not contain a definition for 'ToShortDateString' and no extension method 'ToShortDateString' accepting a first argument of type 'DateTime' could be found (are you missing a using directive or an assembly reference?)

with: ToShortDateString(), ToShortTimeString(), ToLongDateString(), ToLongTimeString(), ToOADate() and maybe more. But ToLocalTime() works fine.

I've checked the sources, those methods exists. https://github.com/dotnet/coreclr/blob/release/1.0.0-rc1/src/mscorlib/src/System/DateTime.cs#L1146

I'm working under OSX El Capitan. I've tested on both version of execution env:

{
  "version": "1.0.0-*",
  "description": "Coreclr DateTime issue",
  "frameworks": {
    "dnxcore50": {
      "dependencies": {
        "Microsoft.CSharp": "4.0.1-beta-23516",
        "System.Console": "4.0.0-beta-23516"
      }
    }
  }
}
using System;

namespace ConsoleApp
{
    public class Program
    {
        public static void Main(string[] args)
        {
          var d = DateTime.Now;
          Console.WriteLine(d.ToString());
          Console.WriteLine(d.ToShortDateString());
        }
    }
}
mikedn commented 8 years ago

.NET Core is a subset of .NET Framework and not all types and methods are available. And not all methods are really needed:

adun commented 8 years ago

I'm aware about ToXString equivalent, I can get satisfied with a workaround. I opened this issue to highlight a cross platform dysfunction.

tarekgh commented 8 years ago

I opened this issue to highlight a cross platform dysfunction

@adun are you talking differences between the desktop and coreclr? if this is the case, we already have many differences between these 2 platforms as we don't support all the desktop public surface. please let me know if you mean something else.

I agree we can expose these APIs just for convenience usages. but we have to be aware we'll have different ways to achieve the same results.

I am listing the workaround explicitly here for the record.

ToShortDateString use DateTime.ToString("d") ToShortTimeString use DateTime.ToString("t") ToLongDateString use DateTime.ToString("D") ToLongTimeString use DateTime.ToString("T")

ToOADate is for OLE which became not popular scenario

tarekgh commented 8 years ago

To the review guys. the suggestion is to expose the following APIs for convenience usage reason only:

namespace System { public struct DateTime : IComparable, IFormattable, IConvertible, ISerializable, IComparable, IEquatable { public String ToShortDateString() { return null; } public String ToShortTimeString() { return null; } public String ToLongDateString() { return null; } public String ToLongTimeString() { return null; } public double ToOADate() { return null; } } }

I am not strangely believe we need to expose ToOADate as it is for OLE automation scenario which is not popular scenario anymore but I listed the API just to get it into the discussion

niemyjski commented 8 years ago

+1

scottdorman commented 8 years ago

This really isn't "for convenience usage reason only", but rather a compatibility issue. Yes, the ToShortxxxString() and ToLongxxxString() methods have a corresponding ToString() method. That's not the issue. The issue is that by removing these methods, you break the upgrade scenario for a lot of projects simply by not having those calls. (By the way, they seem to have been present in earlier versions of .NET Core.)

These methods all still exist in the 1.0 release as well:

https://github.com/dotnet/coreclr/blob/release/1.0.0/src/mscorlib/src/System/DateTime.cs#L1121

So, why are they not being made available?

adrien-constant commented 8 years ago

Agreed, should be available, mostly for compatibility. Using this fix meanwhile :

    public static class DateTimeExtensions
    {
        public static string ToShortDateString(this DateTime dateTime) => dateTime.ToString("d");
        public static string ToShortTimeString(this DateTime dateTime) => dateTime.ToString("t");
        public static string ToLongDateString(this DateTime dateTime) => dateTime.ToString("D");
        public static string ToLongTimeString(this DateTime dateTime) => dateTime.ToString("T");
    }
tarekgh commented 8 years ago

we are currently evaluating the compat work across the board. so most likely we'll bring all missing APIs back.

https://github.com/dotnet/corefx/issues/9643#issuecomment-230459531

scottdorman commented 8 years ago

so most likely we'll bring all missing APIs back.

(I added the emphasis.)

Please don't just simply bring back all of the missing APIs. The direction .NET Core is taking of removing APIs (that are unused, little used or redundant) and trimming the API surface is the right direction overall.

The real problems here are:

For some of these redundant APIs (like Decimal.Round, DateTime.ToShortDate, etc.), the API surface that's added back should simply be forwards to the appropriate call. In fact, some of them at least could be implemented as extension methods (see https://github.com/dotnet/coreclr/issues/2317#issuecomment-229965996). That prevents existing code from breaking while still keeping the implementation details in a single place for maintenance and not appreciably increasing the real API surface area.

I think the approach should be to _add back only those APIs that make sense to add and don't prevent .NET Core from being able to run on all platforms_. If it should be added back but might limit what platforms it can run on, it should be added through a NuGet package that allows the developer to take an explicit dependency on it (and therefor acknowledging that they're limiting the availability).

tarekgh commented 8 years ago

CC @danmosemsft

danmoseley commented 8 years ago

@scottdorman

Please don't just simply bring back all of the missing APIs. The direction .NET Core is taking of removing APIs (that are unused, little used or redundant) and trimming the API surface is the right direction overall.

Broadly, the criteria we're right now using for growing .NET Core API parity include: is it available to Xamarin mobile apps at the base class layer (and if implemented - we implement). That certainly includes many types and members that already have alternatives, or are problematic in one way or another, but it's important to us that we make it easy for existing code to work on .NET Core. We're also valuing "just works without recompilation" which makes it tough to avoid specific API.

When we bring API back to help bring existing codebases to .NET Core, there's still several ways we can try to steer developers from writing new code against them, such as: not referencing by default, or using analyzers or other tools to help flag the code.

Our blog post explains the reasoning. We promised more concrete details and we'll follow up soon with that.

@terrajobst

joperezr commented 7 years ago

This was completed already.