Stable output: Add some more names to anonymous Method classes

[RobQuistNL]

Fix #54

The root issue of the seemingly "random" output is the fact that the sorting method only sorts on items that have a Name decalred. Some Method structs didn't have this by default (such as the unmarshal and enum methods).

I'm not sure if this is a clean solution, but for me it generates solid and stable output so diffs between generated versions are clean :)

What has been done

• Added some arbitrary names to some methods

How to test

• Did 2 generations of slightly different schema files and compared in a diff

To debug, you can add the following code in line 110 in pkg/codegen/model.go (see commit 96737c8):

	for i, decl := range sorted {
		log.Printf("Sorted: %+v %+v", i, decl)
	}

this shows the sorted output.

Todo

Maybe we should add automated tests?

EDIT:
Yep, just found the test data folder 🤡
Looks like this will need a lot of restructuring in that testing data.. but first let me know if this is the right way to go!

[omissis]

uh-uh this solving this issue would be very appreciated @RobQuistNL ! :) I will take a look in the next days and hopefully merge it sooner than later! thanks for putting in the effort!

[RobQuistNL]

Thanks! Yeah it was driving me nuts :+)

I see that there might be one annoying thing here though - the enumValues_ variables end up at the bottom of the generated file.

The actual enums + values do end up in the "proper" position - but I think this might be a rather small change to the code here.

Thanks for the project so far, has helped me greatly!

[RobQuistNL]

Quick question - I got that figured out too - but to make it "sort" properly, the generated interfaces would end up with a different name.

Currently:

var enumValue_Method = []interface{}{
	"GET",
	"POST",
}

Proposed (which sorts it properly)

var Method_enumValues = []interface{}{
	"GET",
	"POST",
}

This is by changing the name. If you'd rather not change the name, we can make an exception in the sorter - if the string begins with enumValues_ - we filter out that piece of string to run the sorting logic. This does change the order, but it doesn't change any previously generated interface names. I think that option would be preferable? I'll implement the latter in a jiffy - LMK if you'd rather see it different.

Commit incoming :)

[RobQuistNL]

hey @omissis sorry to bother you again, but any chance we can get this through?

Do you want me to restructure the test files or something? I'm not sure where they are at the moment but I can try to fix it if you'd like :)

[omissis]

Hey @RobQuistNL I haven't forgotten about it, I will try to give it a look over the weekend

[omissis]

hey @RobQuistNL sorry for not getting to this yet, I will try to take some time over xmas/new year's to review and merge this, as I would really like to have this improvement, too! 🙏

[RobQuistNL]

No worries @omissis , I know how it is 😎
For the time being we're using my own branch so no real hurry from my end :) Take your time and don't forget to enjoy the years' ending too 🥳

[omissis]

Hi! Thanks for the patience. I reviewed the PR and I think it looks quite nice. 👌

I don't have a strong opinion about the CleanNameForSorting workaround: I see the pros and cons of both approaches. I guess we could keep the "hack" for the sake of BC, even though a not too disruptive change like that in a 0.x release doesn't worry me too much to be honest. I reserve the right to change that in a future release for the sake of a simpler codebase 👹

That said, I'd like to ask you a couple things before we merge:

• Fix the linting issues (I already added the suggestions so it should be very quick) 🙏
• Update the go files in the tests folder

The latter item is a bit involved due to the current test setup (I'll see to make it easier in the future). Here's a little walkthrough of how you can do that:

# Remove all generated go code in the test folders
find tests/data -name "*.go" -exec rm {} \;

# Temporarily remove the integration tests
git co tests/unmarshal_test.go tests/validation_test.go

# Run the tests
make test

# Restore the integration tests
git co tests/unmarshal_test.go tests/validation_test.go

I have already done the steps above and checked the diffs, and they look good to me 💯

[RobQuistNL]

Heya, thanks for the review!

I've just tried your suggested steps (I assume your git co is something like a stash? I've removed the files and would unstage the deletion afterwards (rm <file> and git checkout -- <file>)

Either way, every time I run the tests, (also after pulling in main), the tests fail with the following error;

main module (github.com/atombender/go-jsonschema) does not contain package github.com/atombender/go-jsonschema/tests
make: *** [Makefile:220: test] Error 1

[omissis]

Hey @RobQuistNL , thanks for taking the time! yeah, sorry, co is my git shortcut for checkout, and I pasted the second command wrong (that's what you get when you work at night 😅). The problem you are seeing is likely caused by the lack of go.work in your filesystem, you should be able to correct that by running cp go.work.dist go.work.

Once you do that, you can run the (corrected) command I posted above and it should all work.

To recap:

# Create the go.work file, to make both modules available in the current workspace
cp go.work.dist go.work

# Remove all generated go code in the test folders, to allow for the tests to regenerate them
find tests/data -name "*.go" -exec rm {} \;

# Temporarily remove the integration tests, as they depend on the files above
rm tests/unmarshal_test.go tests/validation_test.go

# Run the tests, which will regenerate the missing files we deleted above
make test

# Restore the integration tests
git checkout tests/unmarshal_test.go tests/validation_test.go

# Run the tests once again, to check the two restored ones also pass
make test

[RobQuistNL]

Sweet! Thanks. Yeah, I figured the rm and checkout commands out - I was just missing the go.work file (my bad, didn't even know about that in golang)

Tests now succeed and I've pushed the changes!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (40f48ce) 76.51% compared to head (728e586) 76.58%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   76.51%   76.58%   +0.07%     
==========================================
  Files          24       24              
  Lines        1882     1892      +10     
==========================================
+ Hits         1440     1449       +9     
- Misses        353      354       +1     
  Partials       89       89              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.