Syntax rule-enforcement is inconsistent across languages

[zblach]

Given the following example flatbuffer definition, I see different validation behavior depending on the language that the buffers are being generated for.

namespace Issue;

struct Foo {
    f:uint8;
}
struct Bar {
    b:uint16;
}

union Type {
    Foo, Bar
}

table Example {
    t: Type;
}

This compiles without issue for binary, cpp, java, javascript, typescript, and csharp, but fails for json, golang,dart, python, lua, and rust. I didn't try other combinations, but each rejection yields the same error message -- error: only tables can be union elements in the generated language: Foo.

I found this bug while attempting to validate a schema against itself as a part of a typescript project. I'd suggest fixing this issue by having a common validation path for all languages prior to codegen.

I am using flatc version 1.12.0 on OSX.

[vglavnyy]

This is not the bug. The common path already exists and it is checked before any codegen run:
SupportsAdvancedUnionFeatures():

flatbuffers/src/idl_parser.cpp

Lines 3032 to 3036 in 4e79d12

	if (!SupportsAdvancedUnionFeatures() &&
	(IsStruct(val.union_type) || IsString(val.union_type)))
	return Error(
	"only tables can be union elements in the generated language: " +
	val.name);

flatbuffers/src/idl_parser.cpp

Line 2309 in 4e79d12

bool Parser::SupportsAdvancedUnionFeatures() const {

But this method is inaccessible because it is private method.
Will the public access level solve your issue? For example, Parser::SupportsOptionalScalars(const flatbuffers::IDLOptions &opts) is public method.

The flatc should return a non-zero value on this error it can be used as a signal in a batch script.

Update:
The struct can't be used with unions. This is the common rule for all languages.
Only nullable objects like table or string can be used with union.
Any struct in fbs is a kind of a scalar type.

[zblach]

The struct can't be used with unions. This is the common rule for all languages.
Only nullable objects like table or string can be used with union.
Any struct in fbs is a kind of a scalar type.

I understand that this is a restriction in the language, but for the first set of languages listed, the flatc compiler will generate source code for this definition. This could be a problem if you're bridging languages for which this definition is legal with one where it is not.

[vglavnyy]

I'm sorry, I was wrong. A struct can be used in a union.
Related issue: #4568

[zblach]

If the sample definition is correct, then the error being flagged by the second batch of languages is incorrect. flatc --conform issue.fbs issue.fbs also reports the same validation error.

Regardless of the language of the code being generated, it feels to me like there should be some consistency. Or a warning about using unsupported/experimental features.

[zblach]

Language-feature consistency might be a nice 2.0 feature. One of the reasons flatbuffers are appealing is that they aren't language specific. But when we have mixed feature support based on language, we lose a part of that benefit.

[aardappel]

@zblach our upcoming "2.0" is not necessarily a big overhaul, its mostly a switch to semver, see #6353

Having all features work consistently in all language is of course something we strive for, but is a massive amount of work. It will have to be incremental. And while we're doing that, we're likely to introduce new features, which continue the problem. Implementing features for all languages at once is usually not feasible (nor desirable, the PR would be hard to review).

[zblach]

I understand that, and I appreciate the move to semantic versioning. I'm not really proposing any major functionality changes, I've just been caught off-guard twice now by undocumented feature differences.

[aardappel]

If anything we should document this better. I'll tag this as such, maybe something for @dbaileychess, but this is not a blocker for a 2.0 release.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.