Skip to content
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.

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

Adds -allow-implicit for type annotations #133

Open
wants to merge 2 commits into
base: v1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,12 @@ Default rules:
- `all`: enforce both arguments and return type annotations
- `return`: enforce return type annotations
- `args`: enforce arguments type annotations
- `all-allow-implicit`: enforce argument and return type annotations, but allows arguments with default values to not have an annotation
- `args-allow-implicit`: enforce arguments type annotations, but allows arguments with default values to not have an annotation
- `off`: do not validate (**default**)

- `name-shadowing`: enforces that no two items have the same name (`error | warn | info | off`)

### Code flow rules

Valid values for the rules severity are: `error | warn | info | off`.
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export type RuleBlockIf = 'no-then' | 'then' | 'off';
export type RuleCondition = 'no-group' | 'group' | 'off';
export type RuleFunction = 'no-function' | 'no-sub' | 'auto' | 'off';
export type RuleAAComma = 'always' | 'no-dangling' | 'never' | 'off';
export type RuleTypeAnnotations = 'all' | 'return' | 'args' | 'off';
export type RuleTypeAnnotations = 'all' | 'return' | 'args' | 'off' | 'all-allow-implicit' | 'args-allow-implicit';
export type RuleEolLast = 'always' | 'never' | 'off';
export type RuleColorFormat = 'hash-hex' | 'quoted-numeric-hex' | 'never' | 'off';
export type RuleColorCase = 'upper' | 'lower' | 'off';
Expand Down
154 changes: 99 additions & 55 deletions src/plugins/codeStyle/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,67 +335,111 @@ describe('codeStyle', () => {
const expected = [];
expect(actual).deep.equal(expected);
});
});

it('enforce return type only', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'return',
'no-print': 'off'
}
it('enforce return type only', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'return',
'no-print': 'off'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`05:LINT3010:Strictness: function should declare the return type`
];
expect(actual).deep.equal(expected);
// should only highlight the function name
expect(diagnostics[0].location.range).to.eql(
util.createRange(4, 0, 4, 8)
);
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`05:LINT3010:Strictness: function should declare the return type`
];
expect(actual).deep.equal(expected);
// should only highlight the function name
expect(diagnostics[0].location.range).to.eql(
util.createRange(4, 0, 4, 8)
);
});

it('enforce arguments type only', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'args',
'no-print': 'off'
}
it('enforce arguments type only', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'args',
'no-print': 'off'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`01:LINT3011:Strictness: type annotation required`,
`05:LINT3011:Strictness: type annotation required`,
`13:LINT3011:Strictness: type annotation required`
];
expect(actual).deep.equal(expected);
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`01:LINT3011:Strictness: type annotation required`,
`05:LINT3011:Strictness: type annotation required`
];
expect(actual).deep.equal(expected);
});

it('enforce all annotations', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'all',
'no-print': 'off'
}
it('enforce all annotations', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'all',
'no-print': 'off'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`01:LINT3011:Strictness: type annotation required`,
`05:LINT3010:Strictness: function should declare the return type`,
`05:LINT3011:Strictness: type annotation required`,
`13:LINT3011:Strictness: type annotation required`
];
expect(actual).deep.equal(expected);
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`01:LINT3011:Strictness: type annotation required`,
`05:LINT3010:Strictness: function should declare the return type`,
`05:LINT3011:Strictness: type annotation required`
];
expect(actual).deep.equal(expected);

it('allows implicit type definitions with default values', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations-implicit.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'all-allow-implicit',
'no-print': 'off'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`01:LINT3011:Strictness: type annotation required`,
`05:LINT3010:Strictness: function should declare the return type`,
`05:LINT3011:Strictness: type annotation required`,
`17:LINT3010:Strictness: function should declare the return type`
];
expect(actual).deep.equal(expected);
});


it('allows implicit type definitions with default values for args only', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/type-annotations-implicit.brs'],
rules: {
'named-function-style': 'off',
'anon-function-style': 'off',
'type-annotations': 'args-allow-implicit',
'no-print': 'off'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`01:LINT3011:Strictness: type annotation required`,
`05:LINT3011:Strictness: type annotation required`
];
expect(actual).deep.equal(expected);
});

});

it('enforce no print', async () => {
Expand Down
18 changes: 15 additions & 3 deletions src/plugins/codeStyle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,16 +343,28 @@ export default class CodeStyle implements CompilerPlugin {

// type annotations
if (typeAnnotations !== 'off') {
if (typeAnnotations !== 'args') {
const needsArgType = typeAnnotations.startsWith('args') || typeAnnotations.startsWith('all');
const needsReturnType = typeAnnotations.startsWith('return') || typeAnnotations.startsWith('all');
const allowImplicit = typeAnnotations.includes('allow-implicit');

if (needsReturnType) {
if (hasReturnedValue && !fun.returnTypeExpression) {
diagnostics.push(messages.expectedReturnTypeAnnotation(
// add the error to the function keyword (or just highlight the whole function if that's somehow missing)
fun.tokens.functionType?.location ?? fun.location
));
}
}
if (typeAnnotations !== 'return') {
const missingAnnotation = fun.parameters.find(arg => !arg.typeExpression);
if (needsArgType) {
const missingAnnotation = fun.parameters.find(arg => {
if (!arg.typeExpression) {
if (allowImplicit && arg.defaultValue) {
return false;
}
return true;
}
return false;
});
if (missingAnnotation) {
// only report 1st missing arg annotation to avoid error overload
diagnostics.push(messages.expectedTypeAnnotation(missingAnnotation.location));
Expand Down
19 changes: 19 additions & 0 deletions test/project1/source/type-annotations-implicit.brs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
function test1(arg1, arg2)
print "noop"
end function

function test2(arg1, arg2)
return arg1 + arg2
end function

function test3(arg1 as string, arg2 as string) as string
return arg1 + arg2
end function

function test4(arg1 = 0, arg2 = 1) as integer
return arg1 + arg2
end function

function test5(arg1 = 0 as float, arg2 = 1 as float)
return arg1 + arg2
end function
4 changes: 4 additions & 0 deletions test/project1/source/type-annotations.brs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@ end function
function test3(arg1 as string, arg2 as string) as string
return arg1 + arg2
end function

function test4(arg1 = "hello", arg2 = "world") as string
return arg1 + arg2
end function