Skip to content

Union Return type consistency #1508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TwitchBronBron merged 4 commits into from
Jul 16, 2025
Merged

Union Return type consistency #1508

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
57f44b8
Fine-tunes UnionType return types
markwpearce Jun 9, 2025
cb8ed39
Deals with union of unions and adds transpile tests
markwpearce Jun 9, 2025
9dec2b3
Changes union type tranpilation to give dynamic when using unlike types
markwpearce Jul 4, 2025
c745e9c
Merge branch 'v1' into return_type_consistency
TwitchBronBron Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions src/files/BrsFile.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4254,6 +4254,73 @@ describe('BrsFile', () => {
});
});

describe('union types', () => {

it('use most reducible type for union of primitives', async () => {
await testTranspile(`
function test1() as string or integer
return "hello"
end function

function test2() as double or float
return 1.23
end function

function test3() as integer or longinteger
return 5
end function

function test4() as integer or integer or integer
return 5
end function
`, `
function test1() as dynamic
return "hello"
end function

function test2() as double

@TwitchBronBron TwitchBronBron Jun 9, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think brightscript would coerce an integer into a longinteger, which would technically not be expected behavior. Should this one be dynamic?

@chrisdp what do you think of this one?

@markwpearce markwpearce Jun 9, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is double or float ... that goes to double.

integer or longinteger (test3) -> longinteger

@TwitchBronBron TwitchBronBron Jun 9, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, yeah I was talking more generally about those use cases. Must have selected the wrong one. My concern applies to both integer or longinteger -> longinteger and double or float -> double.

If a function returns a float sometimes, or a double other times, I think the original underlying return type should be respected at runtime. By transpiling to the "more precise" data type, it'll prevent the less precise data type from ever leaving the function. There could be code on the outside looking for "is this a float" or "is this a double" that wouldn't be properly respected with this.

My vote would be for these two situations to transpile to dynamic.

return 1.23
end function

function test3() as longinteger
return 5
end function

function test4() as integer
return 5
end function
`);
});

it('use object for union of object', async () => {
await testTranspile(`
function test1() as object
return 1
end function

function test2() as object or object
return 1
end function

function test3() as object or object or object
return 1
end function
`, `
function test1() as object
return 1
end function

function test2() as object
return 1
end function

function test3() as object
return 1
end function
`);
});
});

describe('callfunc operator', () => {
describe('transpile', () => {
it('does not produce diagnostics on plain roSGNode', () => {
Expand Down
36 changes: 36 additions & 0 deletions src/types/UnionType.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import { isReferenceType, isTypePropertyReferenceType, isUnionType } from '../as
import { TypedFunctionType } from './TypedFunctionType';
import { SymbolTable } from '../SymbolTable';
import { ReferenceType } from './ReferenceType';
import { DoubleType } from './DoubleType';
import { LongIntegerType } from './LongIntegerType';
import { ObjectType } from './ObjectType';


describe('UnionType', () => {
Expand Down Expand Up @@ -190,4 +193,37 @@ describe('UnionType', () => {
expect((returnType as UnionType).types).includes(IntegerType.instance);
});
});

describe('toTypeString', () => {
it('should reduce primitive types to the most generic if reducible', () => {
let ut = new UnionType([FloatType.instance, IntegerType.instance]);
expect(ut.toTypeString()).to.eq('float');
ut = new UnionType([FloatType.instance, IntegerType.instance, DoubleType.instance]);
expect(ut.toTypeString()).to.eq('double');
ut = new UnionType([LongIntegerType.instance, IntegerType.instance]);
expect(ut.toTypeString()).to.eq('longinteger');
ut = new UnionType([DoubleType.instance, LongIntegerType.instance]);
expect(ut.toTypeString()).to.eq('double');
});

it('should reduce object types to object', () => {
let ut = new UnionType([ObjectType.instance, ObjectType.instance]);
expect(ut.toTypeString()).to.eq('object');
ut = new UnionType([ObjectType.instance, ObjectType.instance, ObjectType.instance, ObjectType.instance]);
expect(ut.toTypeString()).to.eq('object');
});

it('should reduce to dynamic if non-reducible', () => {
let ut = new UnionType([FloatType.instance, StringType.instance]);
expect(ut.toTypeString()).to.eq('dynamic');
ut = new UnionType([FloatType.instance, IntegerType.instance, ObjectType.instance]);
expect(ut.toTypeString()).to.eq('dynamic');
ut = new UnionType([LongIntegerType.instance, new InterfaceType('test')]);
expect(ut.toTypeString()).to.eq('dynamic');
ut = new UnionType([ObjectType.instance, ObjectType.instance, StringType.instance, ObjectType.instance]);
expect(ut.toTypeString()).to.eq('dynamic');
ut = new UnionType([ObjectType.instance, new InterfaceType('test')]);
expect(ut.toTypeString()).to.eq('dynamic');
});
});
});
25 changes: 23 additions & 2 deletions src/types/UnionType.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { GetTypeOptions, TypeCompatibilityData } from '../interfaces';
import { isDynamicType, isObjectType, isTypedFunctionType, isUnionType } from '../astUtils/reflection';
import { isDynamicType, isNumberType, isObjectType, isTypedFunctionType, isUnionType } from '../astUtils/reflection';
import { BscType } from './BscType';
import { ReferenceType } from './ReferenceType';
import { addAssociatedTypesTableAsSiblingToMemberTable, findTypeUnion, findTypeUnionDeepCheck, getUniqueType, isEnumTypeCompatible } from './helpers';
import { addAssociatedTypesTableAsSiblingToMemberTable, findTypeUnion, findTypeUnionDeepCheck, getAllTypesFromUnionType, getUniqueType, isEnumTypeCompatible } from './helpers';
import { BscTypeKind } from './BscTypeKind';
import type { TypeCacheEntry } from '../SymbolTable';
import { SymbolTable } from '../SymbolTable';
Expand Down Expand Up @@ -144,7 +144,28 @@ export class UnionType extends BscType {
toString(): string {
return joinTypesString(this.types);
}

/**
* Used for transpilation
*/
toTypeString(): string {
const flattenedTypes = getAllTypesFromUnionType(this);

if (flattenedTypes.length === 0) {
return 'dynamic';
}
if (flattenedTypes.length === 1) {
return flattenedTypes[0].toTypeString();
}

const allNumbers = flattenedTypes.filter(t => isNumberType(t));
if (allNumbers.length === flattenedTypes.length) {
return util.getHighestPriorityType(flattenedTypes).toTypeString();
}
const allObjectType = flattenedTypes.filter(t => isObjectType(t));
if (allObjectType.length === flattenedTypes.length) {
return 'object';
}
return 'dynamic';
}

Expand Down