Undeprecate JSONType member to avoid deprecation warnings

[cschlote]

Undeprecate JSONType members, which create lots of spam-like
deprecation warnings, which can't be fixed by the user.

See discussion on this issue at
https://forum.dlang.org/post/feudrhtxkaxxscwhhhff@forum.dlang.org

[dlang-bot]

Thanks for your pull request and interest in making D better, @cschlote! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	20350	enhancement	JSONType deprecations should be undeprecated

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + phobos#7225"

[FeepingCreature]

The background here is that std.conv.to has to use EnumMembers which necessarily accesses every member, even the deprecated ones. Probably EnumMembers should suppress the deprecation messages, but the language offers no way to interact with deprecations at compiletime in any way.

[CyberShadow]

Yo check this out:

enum OldJSONType : byte
{
	deprecated NULL    = 0,
	deprecated STRING  = 1,
	deprecated INTEGER = 2,
	// ...
}

enum JSONType : OldJSONType
{
	null_   = cast(OldJSONType)0,
	string  = cast(OldJSONType)1,
	integer = cast(OldJSONType)2,
	// ...
}

void main()
{
	import std.stdio;
	auto v = JSONType.NULL;  // works, shows deprecation warning
	writeln(JSONType.null_); // works, no deprecation warnings!
}

Edit: aww, doesn't work with JSONType v = JSONType.NULL;

[CyberShadow]

Try 2:

struct JSONType
{
	enum Type
	{
		null_,
		string,
		integer,
	}
	Type type;
	alias type this;

	enum null_  = JSONType(Type.null_ );
	enum string = JSONType(Type.string);

	deprecated enum NULL   = null_ ;
	deprecated enum STRING = string;

	.string toString() const pure nothrow @nogc
	{
		static immutable names = {
			import std.traits;
			enum enumLength(T) = cast(T)(cast(size_t)T.max + 1);
			.string[enumLength!Type] result;
			foreach (i, t; EnumMembers!Type)
				result[t] = __traits(identifier, EnumMembers!Type[i]);
			return result;
		}();
		return names[type];
	}
}

void main()
{
	import std.stdio;
	JSONType v = JSONType.NULL;  // works, shows deprecation warning
	writeln(JSONType.null_); // works, no deprecation warnings!
}

However, of course this breaks EnumMembers!JSONType, which is something people might be doing.

[FeepingCreature]

Good try, but even the first doesn't work with __traits(allMembers) - and if it does, it shouldn't. :) (Since inherited enum fields should definitely show up in the EnumMembers list.)

We really need a scoped pragma(suppressDeprecations);.

[CyberShadow]

(Since inherited enum fields should definitely show up in the EnumMembers list.)

Inheriting enums in D seems to make it start counting from zero again, not continue the parent enum, so it would make sense for the parents' members to not be present.

[CyberShadow]

Ok ok ok, how about this:

import std.traits;

struct JSONTypeBase
{
	byte value;

	deprecated enum NULL    = cast(JSONType)0;
	deprecated enum STRING  = cast(JSONType)1;
	deprecated enum INTEGER = cast(JSONType)2;
}

enum JSONType : JSONTypeBase
{
	null_   = JSONTypeBase(0),
	string  = JSONTypeBase(1),
	integer = JSONTypeBase(2),
	// ...
}

void main()
{
	import std.stdio;
	JSONType v = JSONType.NULL;      // works, shows deprecation warning
	writeln(JSONType.null_);         // works, no deprecation warnings!
	writeln([EnumMembers!JSONType]); // also works!
}

[wilzbach]

Let's just merge this. The deprecation should have never been merged anyhow as it provides no real-world value and Andrei has rejected a lot more useful things because they deprecated code.
OTOH std.json is like so much of phobos really really old and yet another reason why we need a new version of/updated/different standard library

[CyberShadow]

If we do that then we need to decide what to do with the old enum members.

Do we keep them forever?

Or do we remove them, even though there will be no deprecation warnings for them?

[wilzbach]

(I let this be in master as the master-stable merge is in four days.)

[FeepingCreature]

The deprecation should have never been merged anyhow as it provides no real-world value

Are changes to Phobos to bring the official D standard library in line with the official D coding guidelines no longer desired?

(Disclaimer: I wrote the deprecation)

Submissions to Phobos and other official D source code will follow these guidelines.

I think it's bad development practice to say "Who cares? It's just std.json, everyone knows it's bad, so we can just let it keep being bad." Either remove it or improve it. Or at least don't stand in the way of improvements citing the fact that it's bad as an excuse to stay bad.

[CyberShadow]

This should mention a Bugzilla issue ID in the commit message. I'm sure one has been filed for this problem.

[CyberShadow]

I'm sure one has been filed for this problem.

Maybe not; there are others regarding spurious deprecations, and there's the forum thread, but no bug regarding this particular one.

[CyberShadow]

Are changes to Phobos to bring the D standard library in line with the D coding guidelines no longer desired?

I would say changes to public interfaces which don't bring substantial gain generally don't pull their weight. The enum members previously were ugly and non-conforming, now they're just ugly in a different way (trailing underscores, string is fontified as a type in most editors). And now everyone needs to replace one ugly with another in codebases using std.json.

[FeepingCreature]

At least we only have to get used to one documented ugly instead of two incompatible uglies. 😛

[wilzbach]

Do we keep them forever?

Yes, that's a lot less harmful than this deprecation and Phobos is a conglomerate of historical files either way 🤷‍♂️
We can add a bug report about it that when DMD ever gets fixed so that someone may enable the deprecations again.

Are changes to Phobos to bring the D standard library in line with the D coding guidelines no longer desired?

Where they ever? Standard == stable, not great.
This deprecation had a really bad value/price ratio though. Also, we would need to push hundreds of deprecations if we ever actually wanted to cleanup Phobos, but as has been shown here this is very very unpopular with current users and thus would only ever happen in v3 or a fork.

[FeepingCreature]

To be fair, I've always wished we'd deprecate more. I got up at DConf and said "please, deprecate more things!" 😄

And to be fair, the price was lower when deprecations were broken in EnumMembers. And far as I can see, pretty much nobody complained until that got fixed. (And also if that'd been fixed from the start, I'd never have added the deprecations.)

[cschlote]

Thanks for the interesting discussion, and nice examples how the problem could be worked-around in other ways ;-)

It tested the following:

import std.json, std.stdio;
void main()
{
JSON_TYPE c;
writefln("%s", c);
}
$ dmd test1.d
test1.d(4): Deprecation: alias std.json.JSON_TYPE is deprecated - Use JSONType and the new enum member names

Change JSON_TYPE to JSONType and the message is gone. I presume, that people can read, and change all the member names too. Ok, if they do not, they will find out, when the old member names are removed in ~2.0096.

Ok, that won't work, if JSON_TYPE was changed to JSONType without changing the member names at the same earlier time. In this case people could already use JSONType, but still use the old names for the rest.

I will watch this and related issues, and check the most popular dub packages for them. A fix and some PR should be easy to create.

This should be a better way than using more complicated work-arounds - just fix the code ;-)

[wilzbach]

This should mention a Bugzilla issue ID in the commit message.

Done.

[CyberShadow]

Let's go with this for now to fix the annoying deprecations.

If we find / decide to use a technique of deprecating the old members without impacting enum introspection, then we can re-enable the deprecations and start a new deprecation cycle, then remove them.

[wilzbach]

BTW I just saw that we missed 2.089 by a few hours, but there's nothing that we can do it about this anymore and at least as this in stable it will be part of the next patch release.

[ghost]

ping?