bcp47_language_tag doesn't fail on some non-BCP47 tags

[bfabio]

• I have looked at the documentation here first?
• I have looked at the examples provided that may showcase my question here?

Package version eg. v9, v10:

v10

Issue, Question or Enhancement:

When using bcp47_language_tag for validation, some non-BCP47 tags such as "eng" or "en_US" are passing as valid.

‎isBCP47LanguageTag() uses golang.org/x/text/language's Parse and its documentation says:

[snip]
It accepts tags in the BCP 47 format and extensions to this standard defined in https://www.unicode.org/reports/tr35/#Unicode_Language_and_Locale_Identifiers.

Code sample, to showcase or reproduce:

I expect both of these to fail, but they don't:

package main

import (
	"fmt"
	"github.com/go-playground/validator/v10"
)

func main() {
	validate := validator.New()

	err := validate.Var("en_US", "bcp47_language_tag")
	if err != nil {
		fmt.Println(err.Error())
		return
	}

	err = validate.Var("eng", "bcp47_language_tag")
	if err != nil {
		fmt.Println(err.Error())
		return
	}
}

[shihanng]

I think golang.org/x/text/language's Parse is based on Unicode Locale Data Markup Language (LDML)'s Unicode Language and Locale Identifiers which is based on BCP47 (but they are not strictly the same). E.g., Unicode Language and Locale Identifiers allow the underscore _ to be used as a separator.

sep = [-_] ;

But not BCP47:

 langtag       = language
                 ["-" script]
                 ["-" region]
                 *("-" variant)
                 *("-" extension)
                 ["-" privateuse]

There is a section called BCP 47 Conformance which reads:

It allows certain syntax for backwards compatibility (not BCP 47-compatible):

• The "_" character for field separator characters, as well as the "-" used in

[briskt]

As I compare validator docs, the specs, and this issue writeup, it seems to me that the differing standards would have to be implemented as separate validations. The existing bcp47_language_tag is documented as follows:

This validates that a string value is a valid BCP 47 language tag, as parsed by language.Parse.
More information on https://pkg.go.dev/golang.org/x/text/language

which, as @shihanng identified, and the Go docs describe, uses the Unicode extension to BCP 47. I think the stricter standard would have to be added as a separate validator to avoid breaking compatibility. That is, if the stricter validation is even necessary. Do you still have this requirement, @bfabio?

[bfabio]

As I compare validator docs, the specs, and this issue writeup, it seems to me that the differing standards would have to be implemented as separate validations. The existing bcp47_language_tag is documented as follows:

This validates that a string value is a valid BCP 47 language tag, as parsed by language.Parse.
More information on https://pkg.go.dev/golang.org/x/text/language

Yes, it is documented but I see the use language.Parse as a implementation detail that leaked to the user. I think the issue I saw stems from https://www.unicode.org/reports/tr35/#Compatibility_with_Older_Identifiers, and the legacy support language.Parse has for older LDML (https://unicode.org/reports/tr35/) as @shihanng hinted.

which, as @shihanng identified, and the Go docs describe, uses the Unicode extension to BCP 47. I think the stricter standard would have to be added as a separate validator to avoid breaking compatibility. That is, if the stricter validation is even necessary. Do you still have this requirement, @bfabio?

I do. In publiccode.yml we have a few fields defined as containing strict BCP-47 (such as this one, so our parser must implement it (italia/publiccode-parser-go#47).

I agree that it could be a separate validator at this point (bcp47_strict_language_tag ?), unless we want to allow some breakage (which I don't know how many users will bite, I assume not many because it's such an exotic nuance) and use bcp47_language_tag and add a new bcp47_with_unicode_extensions_language_tag.

[zemzale]

I agree that it could be a separate validator at this point (bcp47_strict_language_tag ?), unless we want to allow some breakage (which I don't know how many users will bite, I assume not many because it's such an exotic nuance) and use bcp47_language_tag and add a new bcp47_with_unicode_extensions_language_tag.

We are strictly looking to not break things, even if that is an exotic nuance. So if this is added as a tag then it would have to be added as some sort of bcp47_strict_language_tag (not sure of the naming)

[bfabio]

@zemzale makes perfect sense and it would be fine for me