x.json2: replace x.json2.decode() with x.json2.decoder2.decode()

[dy-tea]

Original pr with @[deprecated_after] moved to #25480

[huly-for-github]

Connected to Huly®: V_0.6-26156

[dy-tea]

Ah that's an issue, now that the deprecation is working it's giving errors in all those cases.

[JalonSolov]

Rip 'em out. @spytheman does it manually, occasionally - we don't have automation to remove completely deprecated things.

[dy-tea]

I will rip it out and just add it to this pr then.

[dy-tea]

vlib/x/json2/tests/decode_map_of_map_test.v appears to have a runtime error when using decoder2.

/tmp/v_1000/tsession_7ff32091a740_01K77ZGP4W5NGGYPW048KY0JGP/decode_struct_test.01K77ZM2EBP6GNEWXZC:19137: at x__json2__decoder2__Decoder_decode_string_T_string: RUNTIME ERROR: invalid memory access
/tmp/v_1000/tsession_7ff32091a740_01K77ZGP4W5NGGYPW048KY0JGP/decode_struct_test.01K77ZM2EBP6GNEWXZC:16981: by x__json2__decoder2__Decoder_decode_value_T_string
/tmp/v_1000/tsession_7ff32091a740_01K77ZGP4W5NGGYPW048KY0JGP/decode_struct_test.01K77ZM2EBP6GNEWXZC:18839: by x__json2__decoder2__Decoder_decode_value_T_main__StructTypeSub
/tmp/v_1000/tsession_7ff32091a740_01K77ZGP4W5NGGYPW048KY0JGP/decode_struct_test.01K77ZM2EBP6GNEWXZC:12439: by x__json2__decoder2__Decoder_decode_value_T_main__StructType_T_main__StructTypeSub
/tmp/v_1000/tsession_7ff32091a740_01K77ZGP4W5NGGYPW048KY0JGP/decode_struct_test.01K77ZM2EBP6GNEWXZC:10796: by x__json2__decoder2__decode_T_main__StructType_T_main__StructTypeSub
/tmp/v_1000/tsession_7ff32091a740_01K77ZGP4W5NGGYPW048KY0JGP/decode_struct_test.01K77ZM2EBP6GNEWXZC:21331: by main__test_types
/tmp/v_1000/tsession_7ff32091a740_01K77ZGP4W5NGGYPW048KY0JGP/decode_struct_test.01K77ZM2EBP6GNEWXZC:22646: by main

[dy-tea]

Potentially could move decoder2 up a directory to json2 (since the tooling also depends on it).

[JalonSolov]

Separate PR. Only change what's needed for the intent of the PR each time. Keeps them smaller, as well as making them quicker/easier to review.

Moving those up a level will definitely deserve a separate PR. You have to move the current files up, create "stub" files that give a deprecation message, and import the files that were moved, etc., etc.

[spytheman]

Imho it should always require the deprecated tag, because of the message; using just deprecated_after does not provide a way to add one

[spytheman]

Didn't add a test as this would change depending on the date which would be tricky.

use a far future deprecation date or a date in the past, depending on what you need to test

[spytheman]

There is a script ./cmd/tools/show_ancient_deprecations.v that can be used to show what needs to be cleaned up, which I run ~3-4 times/year.

[spytheman]

I think that this PR should be split - the parser change and a test for it should be 1, and the json changes should be an entirely separate PR (potentially first to be merged).

[spytheman]

Only change what's needed for the intent of the PR each time. Keeps them smaller, as well as making them quicker/easier to review.

yes, 100%

[spytheman]

@Larsimusrex what do you think about the x.json2 changes?

[Larsimusrex]

So it just removes json2.decode? It doesn't really matter to me as I'm working on decoder2 anyways, but it might be confusing/inconvenient for the people who use json2. It would be better to just replace it with decoder2 outright, I just hadn't had time to do that yet. Also for the json2 tests most of them a redundant, as they were already rewritten for decoder2.

[dy-tea]

I think that this PR should be split - the parser change and a test for it should be 1, and the json changes should be an entirely separate PR (potentially first to be merged).

Yes I can do the json changes in a seperate pr but they will have to be merged first (as the json decode function is a hard error).

So it just removes json2.decode? It doesn't really matter to me as I'm working on decoder2 anyways, but it might be confusing/inconvenient for the people who use json2. It would be better to just replace it with decoder2 outright, I just hadn't had time to do that yet. Also for the json2 tests most of them a redundant, as they were already rewritten for decoder2.

I think the best thing would be to replace json2.decode outright, some tests use json2.raw_decode and there is one that creates a Parser so those need to be changed (or removed).

What I will do for now is remove the @[deprecated_after] commit from this pr and open another one (although it is only a single line change it makes it clearer). Then we can discuss about whether or not the @[deprecated] tag should be a hard requirement for @[deprecated_after] then.

Edit: opened a seperate pr for @[deprecated_after] here #25480.

[dy-tea]

Docs CI is failing because of a bug in decoder2:
https://play.vlang.io/p/165805379c

Will open an issue.

[enghitalo]

IMHO, in this case, just replacing it is much better

[dy-tea]

IMHO, in this case, just replacing it is much better

Yeah that's what I'm going ahead and doing right now as it seemed the most sensible option.

[JalonSolov]

The stub module isn't in the spirit of deprecations... a (possibly?) better way would be to make that module import json2 itself, then call the functions from there, instead of returning an error.

Just returning an error breaks compiles as badly as removing routines directly.

Also, it might be good to choose a new deprecation date (as in today), since nobody has been notified about the previous date before, and it is already past the date where those deprecations would change from warnings to errors - another hard break in compiles.

[dy-tea]

Will wait for #25489 as this should fix the docs CI and then I can update the test in there to use the updated decoder2.

[dy-tea]

Rebased, CI should pass now.

[spytheman]

@dy-tea thank you 🙇🏻

[spytheman]

there is no need to deprecate a non pub function (since people were not able to depend on it)

[dy-tea]

Ah that's right.

[spytheman]

should the date here be 2025-03-18 or the current one?

[dy-tea]

I believe it was deprecated earlier, I just moved it to stub.v