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

Implemented no-todo code style rule. #56

Merged
merged 2 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ Default rules:
"anon-function-style": "auto",
"aa-comma-style": "no-dangling",
"no-print": "off",
"no-todo": "off",
"todo-pattern": "TODO|todo|FIXME",

"type-annotations": "off",

Expand Down Expand Up @@ -185,6 +187,10 @@ Default rules:

- `no-print`: prevent usage of `print` statements in code (`error | warn | info | off`)

- `no-todo`: prevent usage of `todo` comments in code (`error | warn | info | off`)

- `todo-pattern`: string regex pattern to determine what a TODO is. default is `TODO|todo|FIXME` (**do not** include surrounding `/` characters).

### Strictness rules

- `type-annotations`: validation of presence of `as` type annotations, for function
Expand Down
4 changes: 4 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export type BsLintConfig = Pick<BsConfig, 'project' | 'rootDir' | 'files' | 'cwd
'aa-comma-style'?: RuleAAComma;
'type-annotations'?: RuleTypeAnnotations;
'no-print'?: RuleSeverity;
'no-todo'?: RuleSeverity;
// Will be transformed to RegExp type when program context is created.
'todo-pattern'?: string;
};
globals?: string[];
ignores?: string[];
Expand All @@ -59,6 +62,7 @@ export interface BsLintRules {
aaCommaStyle: RuleAAComma;
typeAnnotations: RuleTypeAnnotations;
noPrint: BsLintSeverity;
noTodo: BsLintSeverity;
}

export { Linter };
Expand Down
10 changes: 9 additions & 1 deletion src/plugins/codeStyle/diagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export enum CodeStyleError {
TypeAnnotation = 'LINT3011',
NoPrint = 'LINT3012',
AACommaFound = 'LINT3013',
AACommaMissing = 'LINT3014'
AACommaMissing = 'LINT3014',
NoTodo = 'LINT3015'
}

const CS = 'Code style:';
Expand Down Expand Up @@ -113,6 +114,13 @@ export const messages = {
message: `${CS} Avoid using direct Print statements`,
range
}),
noTodo: (range: Range, severity: DiagnosticSeverity) => ({
severity: severity,
code: CodeStyleError.NoTodo,
source: 'bslint',
message: `${CS} Avoid using TODO comments`,
range
}),
removeAAComma: (range: Range) => ({
severity: DiagnosticSeverity.Error,
code: CodeStyleError.AACommaFound,
Expand Down
36 changes: 36 additions & 0 deletions src/plugins/codeStyle/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,42 @@ describe('codeStyle', () => {
expect(actual).deep.equal(expected);
});

describe('enforce no todo', () => {
it('default todo pattern', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/no-todo.brs'],
rules: {
'no-todo': 'error'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`02:LINT3015:Code style: Avoid using TODO comments`,
`04:LINT3015:Code style: Avoid using TODO comments`,
`06:LINT3015:Code style: Avoid using TODO comments`,
`08:LINT3015:Code style: Avoid using TODO comments`,
'10:LINT3015:Code style: Avoid using TODO comments',
'12:LINT3015:Code style: Avoid using TODO comments'
];
expect(actual).deep.equal(expected);
});

it('modified todo pattern', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/no-todo.brs'],
rules: {
'no-todo': 'error',
'todo-pattern': 'PLEASEFIX'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = ['19:LINT3015:Code style: Avoid using TODO comments'];
expect(actual).deep.equal(expected);
});
});

describe('AA style', () => {
it('collects wrapping AA members indexes', () => {
const { statements } = Parser.parse(`
Expand Down
11 changes: 10 additions & 1 deletion src/plugins/codeStyle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ export default class CodeStyle {

const diagnostics: (Omit<BsDiagnostic, 'file'>)[] = [];
const { severity, fix } = this.lintContext;
const { inlineIfStyle, blockIfStyle, conditionStyle, noPrint, aaCommaStyle } = severity;
const { inlineIfStyle, blockIfStyle, conditionStyle, noPrint, noTodo, aaCommaStyle } = severity;
const validatePrint = noPrint !== DiagnosticSeverity.Hint;
const validateTodo = noTodo !== DiagnosticSeverity.Hint;
const validateInlineIf = inlineIfStyle !== 'off';
const disallowInlineIf = inlineIfStyle === 'never';
const requireInlineIfThen = inlineIfStyle === 'then';
Expand Down Expand Up @@ -85,6 +86,14 @@ export default class CodeStyle {
if (validateAAStyle) {
this.validateAAStyle(e, aaCommaStyle, diagnostics);
}
},
CommentStatement: e => {
if (validateTodo) {
console.log('this.lintContext', this.lintContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

No logs

if (this.lintContext.todoPattern.test(e.text)) {
diagnostics.push(messages.noTodo(e.range, noTodo));
}
}
}
}), { walkMode: walkExpressions ? WalkMode.visitAllRecursive : WalkMode.visitStatementsRecursive });

Expand Down
6 changes: 5 additions & 1 deletion src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ function tryLoadConfig(filename: string): BsLintConfig | undefined {
export interface PluginContext {
program: Readonly<Program>;
severity: Readonly<BsLintRules>;
// TODO: Is this the right place to put this?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes! Though I'm not sure how well it will scale as rules are added...

todoPattern: Readonly<RegExp>;
globals: string[];
ignores: (file: BscFile) => boolean;
fix: Readonly<boolean>;
Expand All @@ -118,6 +120,7 @@ export function createContext(program: Program): PluginWrapperContext {
return {
program: program,
severity: rulesToSeverity(rules),
todoPattern: rules['todo-pattern'] ? new RegExp(rules['todo-pattern']) : /TODO|todo|FIXME/,
globals,
ignores: (file: BscFile) => {
return !file || ignorePatterns.some(pattern => minimatch(file.pathAbsolute, pattern));
Expand Down Expand Up @@ -152,7 +155,8 @@ function rulesToSeverity(rules: BsLintConfig['rules']) {
anonFunctionStyle: rules['anon-function-style'],
aaCommaStyle: rules['aa-comma-style'],
typeAnnotations: rules['type-annotations'],
noPrint: ruleToSeverity(rules['no-print'])
noPrint: ruleToSeverity(rules['no-print']),
noTodo: ruleToSeverity(rules['no-todo'])
};
}

Expand Down
20 changes: 20 additions & 0 deletions test/project1/source/no-todo.brs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
sub myFunc()
' TODO: Fix this function

' todo: please fix this

' todo: fix this indentation

myLabel = CreateObject("roSGNode", "Label") ' todo: add font to label

' FIXME: WE'RE IN ALL CAPS NOW FOR GOD'S SAKE FIX THIS!!!

' TODO: This is a multiline todo
' because sometimes we need to overexplain things.

' and this is a regular comment!

' another one!

' PLEASEFIX: a different pattern
end sub