Go: Make models-as-data package column match any version without "$ANYVERSION"

[owen-mc]

Currently a models-as-data package column like "github.com/a/b/c/d" will match the import path for any major version, but only if the major version suffix (like "/v2") goes at the end ("github.com/a/b/c/d/v2"). If the module path is "github.com/a/b" then we actually want to match ""github.com/a/b/v2/c/d". At the moment this is possible by putting "github.com/a/b/$ANYVERSION/c/d" in the package column.

This PR changes that so we will match major version suffixes at the correct place in the package path.

There are times when we only want to match a specific version of a package. If the package column contains a major version suffix (like "/v2") or if it starts with "fixed-version:" then we don't do any version matching (and prefix is removed).

[owen-mc]

I don't think this needs a change note because models-as-data is in beta for Go. I am happy to add one if someone wants, though.

[github-actions]

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

go

Generated file changes for go

• Changes to framework-coverage-go.rst:

-    Others,"``github.com/go-jose/go-jose/$ANYVERSION/jwt``, ``gopkg.in/square/go-jose.v2/jwt``",,8,2
+    Others,"``github.com/go-jose/go-jose/jwt``, ``gopkg.in/square/go-jose.v2/jwt``",,8,2

• Changes to framework-coverage-go.csv:

- github.com/go-jose/go-jose/$ANYVERSION/jwt,1,,4,,1,,4,
+ github.com/go-jose/go-jose/jwt,1,,4,,1,,4,
- github.com/go-pg/pg/$ANYVERSION/orm,,,6,,,,6,
+ github.com/go-pg/pg/orm,,,6,,,,6,
- github.com/golang/protobuf/$ANYVERSION/proto,,,4,,,,4,
+ github.com/golang/protobuf/proto,,,4,,,,4,
- github.com/kataras/iris/$ANYVERSION/middleware/jwt,1,,,1,,,,
+ github.com/kataras/iris/middleware/jwt,1,,,1,,,,
- github.com/lestrrat-go/jwx/$ANYVERSION/jwk,1,,,1,,,,
+ github.com/lestrrat-go/jwx/jwk,1,,,1,,,,
- github.com/sendgrid/sendgrid-go/$ANYVERSION/helpers/mail,,,1,,,,1,
+ github.com/sendgrid/sendgrid-go/helpers/mail,,,1,,,,1,
- golang.org/x/net/$ANYVERSION/html,,,16,,,,16,
+ golang.org/x/net/html,,,16,,,,16,
- google.golang.org/protobuf/$ANYVERSION/internal/encoding/text,,,1,,,,1,
+ google.golang.org/protobuf/internal/encoding/text,,,1,,,,1,
- google.golang.org/protobuf/$ANYVERSION/internal/impl,,,2,,,,2,
+ google.golang.org/protobuf/internal/impl,,,2,,,,2,
- google.golang.org/protobuf/$ANYVERSION/proto,,,8,,,,8,
+ google.golang.org/protobuf/proto,,,8,,,,8,
- google.golang.org/protobuf/$ANYVERSION/reflect/protoreflect,,,1,,,,1,
+ google.golang.org/protobuf/reflect/protoreflect,,,1,,,,1,
- k8s.io/apimachinery/$ANYVERSION/pkg/runtime,,,47,,,,47,
+ k8s.io/apimachinery/pkg/runtime,,,47,,,,47,

[owen-mc]

DCA shows nothing interesting.

[aschackmull]

Perhaps slightly simpler:

Suggested change

	if p.suffix(p.length() - thisVersion.length()) = thisVersion
	then result = p.prefix(p.length() - 12)
	else
	p = result + thisVersion
	or
	not p = any(string s) + thisVersion and

[aschackmull]

qldoc terminates a bit abruptly.

[owen-mc]

There was a bug in the regex where it had an unwanted $ at the end. This was introduced by copilot, and I didn't check it carefully enough. I could have just copy-pasted it from the other location where I had the correct regex. (Should I try to only specify the regex in one place?)

[aschackmull]

Should I try to only specify the regex in one place?

If you want - I don't have a strong opinion on that. OTOH, if you need to account for specifically not matching something like /v2foo by e.g. using lookahead, then it starts to make more sense to share the regex, as it'll be a bit more complicated.

[owen-mc]

Good idea about using lookahead. I don't think it's very likely that we'll come across that, but it's better to be safe than sorry.

[owen-mc]

The QA results look fine so I'm marking this ready to review.

[smowton]

Looks sensible except that the syntax is a bit weird: the $ANYVERSION was like a substitution variable, a place where /v2 could occur. The $THISVERSION comes at the end and substitutes for nothing. How about instead a prefix that syntactically can't occur in a URL, like versioned:blah.com/pkgname/v2/subpkg?

[owen-mc]

I like the suggestion. I think we could be a bit clearer than "versioned". I think "fixed-version" would be clearer, so I'll implement that.

[owen-mc]

Currently, "fixed-version:" is only needed for v0 and v1 (because having "/v3" in the package column will have the same effect). Here's an idea, to get more functionality:

• if you put "github.com/a/b/v4/c" then we model v4 and higher. (I think this is the most common case.)
• to model only v4, you have to put "fixed-version:github.com/a/b/v4/c"

Or maybe introduce "min-version:github.com/a/b/v4/c" to match version 4 and higher, and a corresponding "max-version:".

Alternatively, we could use the existing range syntax, like "v[2..4]", "v[..4]" and "v[3..]" (though I don't think we allow open-ended ranges in go at the moment).

I don't think we need to implement any of these now, but we could in future if we find that we need it.

[smowton]

In future sounds wise :) Fixed = fixed, default = any seems fine for now.

[owen-mc]

The test failure is unrelated and will be fixed tomorrow. I think this is ready to merge once I've rerun the tests tomorrow.

[smowton]

Looks good -- suggest editing the PR title and description (and commit messages?) for future readers' sake.