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 newline-last rule #58

Merged
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,15 @@ Default rules:
- `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).

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

- `eol-last`: enforces at least one newline (or absence thereof) at the end of non-empty files
arturocuya marked this conversation as resolved.
Show resolved Hide resolved

- `always`: enforces that files end with a newline (**default**)
- `never` enforces that files do not end with a newline
- `off`: do not validate

### Strictness rules

- `type-annotations`: validation of presence of `as` type annotations, for function
Expand Down Expand Up @@ -257,6 +264,7 @@ Running `bslint` with `--fix` parameter will attempt to fix common code-style is
- Using/missing parenthesis around `if/while` conditions.
- Adding/removing Associative Array (AA) literals' commas where needed.
- Case sensitivity (align with first occurence)
- Adding/removing newlines the end of non-empty files.

## Usage checking (approximative)

Expand Down
3 changes: 3 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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 RuleEolLast = 'always' | 'never' | 'off';

export type BsLintConfig = Pick<BsConfig, 'project' | 'rootDir' | 'files' | 'cwd' | 'watch'> & {
lintConfig?: string;
Expand All @@ -37,6 +38,7 @@ export type BsLintConfig = Pick<BsConfig, 'project' | 'rootDir' | 'files' | 'cwd
'no-todo'?: RuleSeverity;
// Will be transformed to RegExp type when program context is created.
'todo-pattern'?: string;
'eol-last'?: RuleEolLast;
};
globals?: string[];
ignores?: string[];
Expand Down Expand Up @@ -64,6 +66,7 @@ export interface BsLintRules {
noPrint: BsLintSeverity;
noTodo: BsLintSeverity;
noStop: BsLintSeverity;
eolLast: RuleEolLast;
}

export { Linter };
Expand Down
19 changes: 18 additions & 1 deletion src/plugins/codeStyle/diagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export enum CodeStyleError {
AACommaFound = 'LINT3013',
AACommaMissing = 'LINT3014',
NoTodo = 'LINT3015',
NoStop = 'LINT3016'
NoStop = 'LINT3016',
EolLastMissing = 'LINT3017',
EolLastFound = 'LINT3018'
}

const CS = 'Code style:';
Expand Down Expand Up @@ -142,5 +144,20 @@ export const messages = {
source: 'bslint',
message: `Add comma after the expression`,
range
}),
addEolLast: (range: Range, preferredEol: string) => ({
severity: DiagnosticSeverity.Error,
code: CodeStyleError.EolLastMissing,
source: 'bslint',
message: `${CS} File should end with a newline`,
range,
data: { preferredEol }
}),
removeEolLast: (range: Range) => ({
severity: DiagnosticSeverity.Error,
code: CodeStyleError.EolLastFound,
source: 'bslint',
message: `${CS} File should not end with a newline`,
range
})
};
175 changes: 160 additions & 15 deletions src/plugins/codeStyle/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,77 @@ describe('codeStyle', () => {
expect(actual).deep.equal(expected);
});

describe('enforce eol at end of file', () => {
it('eol always', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/no-eol-last.brs'],
rules: {
'eol-last': 'always'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`03:LINT3017:Code style: File should end with a newline`
];
expect(actual).deep.equal(expected);
});

it('eol never', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/eol-last.brs'],
rules: {
'eol-last': 'never'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [
`03:LINT3018:Code style: File should not end with a newline`
];
expect(actual).deep.equal(expected);
});

it('empty file', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/empty.brs'],
rules: {
'eol-last': 'always'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [];
expect(actual).deep.equal(expected);
});

it('off without eol', async() => {
const diagnostics = await linter.run({
...project1,
files: ['source/no-eol-last.brs'],
rules: {
'eol-last': 'off'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [];
expect(actual).deep.equal(expected);
});

it('off with eol', async() => {
const diagnostics = await linter.run({
...project1,
files: ['source/main.brs'],
rules: {
'eol-last': 'off'
}
});
const actual = fmtDiagnostics(diagnostics);
const expected = [];
expect(actual).deep.equal(expected);
});
});

describe('AA style', () => {
it('collects wrapping AA members indexes', () => {
const { statements } = Parser.parse(`
Expand Down Expand Up @@ -527,25 +598,28 @@ describe('codeStyle', () => {
});

describe('fix', () => {
// Filenames (without the extension) that we want to copy with a "-temp" suffix
const tmpFileNames = [
'function-style',
'if-style',
'aa-style',
'eol-last',
'no-eol-last',
'single-line'
];

beforeEach(() => {
fs.copyFileSync(
`${project1.rootDir}/source/function-style.brs`,
`${project1.rootDir}/source/function-style-temp.brs`
);
fs.copyFileSync(
`${project1.rootDir}/source/if-style.brs`,
`${project1.rootDir}/source/if-style-temp.brs`
);
fs.copyFileSync(
`${project1.rootDir}/source/aa-style.brs`,
`${project1.rootDir}/source/aa-style-temp.brs`
);
tmpFileNames.forEach(filename => fs.copyFileSync(
`${project1.rootDir}/source/${filename}.brs`,
`${project1.rootDir}/source/${filename}-temp.brs`
));
});

afterEach(() => {
fs.unlinkSync(`${project1.rootDir}/source/function-style-temp.brs`);
fs.unlinkSync(`${project1.rootDir}/source/if-style-temp.brs`);
fs.unlinkSync(`${project1.rootDir}/source/aa-style-temp.brs`);
// Clear temp files
tmpFileNames.forEach(filename => fs.unlinkSync(
`${project1.rootDir}/source/${filename}-temp.brs`
));
});

it('replaces `sub` with `function`', async () => {
Expand Down Expand Up @@ -774,5 +848,76 @@ describe('codeStyle', () => {
const expectedSrc = fs.readFileSync(`${project1.rootDir}/source/aa-style-nodangling.brs`).toString();
expect(actualSrc).to.equal(expectedSrc);
});

it('adds eol last', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/no-eol-last-temp.brs'],
rules: {
'eol-last': 'always'
},
fix: true
});

const actual = fmtDiagnostics(diagnostics);
const expected = [];

expect(actual).deep.equal(expected);

expect(lintContext.pendingFixes.size).equals(1);
await lintContext.applyFixes();
expect(lintContext.pendingFixes.size).equals(0);

const actualSrc = fs.readFileSync(`${project1.rootDir}/source/no-eol-last-temp.brs`).toString();
const expectedSrc = fs.readFileSync(`${project1.rootDir}/source/eol-last.brs`).toString();

expect(actualSrc).to.equal(expectedSrc);
});

it('adds eol last to single line file', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/single-line-temp.brs'],
rules: {
'eol-last': 'always'
},
fix: true
});

const actual = fmtDiagnostics(diagnostics);
const expected = [];

expect(actual).deep.equal(expected);

expect(lintContext.pendingFixes.size).equals(1);
await lintContext.applyFixes();
expect(lintContext.pendingFixes.size).equals(0);

const actualSrc = fs.readFileSync(`${project1.rootDir}/source/single-line-temp.brs`).toString();
const expectedSrc = fs.readFileSync(`${project1.rootDir}/source/single-line-eol.brs`).toString();
expect(actualSrc).to.equal(expectedSrc);
});

it('removes eol last', async () => {
const diagnostics = await linter.run({
...project1,
files: ['source/eol-last-temp.brs'],
rules: {
'eol-last': 'never'
},
fix: true
});
const actual = fmtDiagnostics(diagnostics);
const expected = [];
expect(actual).deep.equal(expected);

expect(lintContext.pendingFixes.size).equals(1);
await lintContext.applyFixes();
expect(lintContext.pendingFixes.size).equals(0);

const actualSrc = fs.readFileSync(`${project1.rootDir}/source/eol-last-temp.brs`).toString();
const expectedSrc = fs.readFileSync(`${project1.rootDir}/source/no-eol-last.brs`).toString();
expect(actualSrc).to.equal(expectedSrc);
});
});
});
44 changes: 43 additions & 1 deletion src/plugins/codeStyle/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class CodeStyle {

const diagnostics: (Omit<BsDiagnostic, 'file'>)[] = [];
const { severity, fix } = this.lintContext;
const { inlineIfStyle, blockIfStyle, conditionStyle, noPrint, noTodo, noStop, aaCommaStyle } = severity;
const { inlineIfStyle, blockIfStyle, conditionStyle, noPrint, noTodo, noStop, aaCommaStyle, eolLast } = severity;
const validatePrint = noPrint !== DiagnosticSeverity.Hint;
const validateTodo = noTodo !== DiagnosticSeverity.Hint;
const validateNoStop = noStop !== DiagnosticSeverity.Hint;
Expand All @@ -37,6 +37,48 @@ export default class CodeStyle {
const requireConditionGroup = conditionStyle === 'group';
const validateAAStyle = aaCommaStyle !== 'off';
const walkExpressions = validateAAStyle;
const validateEolLast = eolLast !== 'off';
const disallowEolLast = eolLast === 'never';

// Check if the file is empty by going backwards from the last token,
// meaning there are tokens other than `Eof` and `Newline`.
const { tokens } = file.parser;
let isFileEmpty = true;
for (let i = tokens.length - 1; i >= 0; i--) {
if (tokens[i].kind !== TokenKind.Eof &&
tokens[i].kind !== TokenKind.Newline) {
isFileEmpty = false;
break;
}
}

// Validate `eol-last` on non-empty files
if (validateEolLast && !isFileEmpty) {
const penultimateToken = tokens[tokens.length - 2];
if (disallowEolLast) {
if (penultimateToken?.kind === TokenKind.Newline) {
diagnostics.push(messages.removeEolLast(penultimateToken.range));
}
} else if (penultimateToken?.kind !== TokenKind.Newline) {
// Set the preferredEol as the last newline.
// The fix function will handle the case where preferredEol is undefined.
// This could happen in valid single line files, like:
// `sub foo() end sub\EOF`
let preferredEol;
for (let i = tokens.length - 1; i >= 0; i--) {
if (tokens[i].kind === TokenKind.Newline) {
preferredEol = tokens[i].text;
}
}

diagnostics.push(
messages.addEolLast(
penultimateToken.range,
preferredEol
)
);
}
}

file.ast.walk(createVisitor({
IfStatement: s => {
Expand Down
28 changes: 28 additions & 0 deletions src/plugins/codeStyle/styleFixes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { BscFile, BsDiagnostic, FunctionExpression, GroupingExpression, IfStatement, isIfStatement, Position, Range, TokenKind, WhileStatement } from 'brighterscript';
import { ChangeEntry, comparePos, insertText, replaceText } from '../../textEdit';
import { CodeStyleError } from './diagnosticMessages';
import { platform } from 'process';

export function extractFixes(
addFixes: (file: BscFile, changes: ChangeEntry) => void,
Expand Down Expand Up @@ -36,6 +37,10 @@ export function getFixes(diagnostic: BsDiagnostic): ChangeEntry {
return removeAAComma(diagnostic);
case CodeStyleError.AACommaMissing:
return addAAComma(diagnostic);
case CodeStyleError.EolLastMissing:
return addEolLast(diagnostic);
case CodeStyleError.EolLastFound:
return removeEolLast(diagnostic);
default:
return null;
}
Expand Down Expand Up @@ -142,3 +147,26 @@ function replaceFunctionTokens(diagnostic: BsDiagnostic, token: string) {
]
};
}

function addEolLast(diagnostic: BsDiagnostic): ChangeEntry {
return {
diagnostic,
changes: [
insertText(
diagnostic.range.end,
// In single line files, the `preferredEol` cannot be determined
// e.g: `sub foo() end sub\EOF`
diagnostic.data.preferredEol ?? (platform.toString() === 'win32' ? '\r\n' : '\n')
)
]
};
}

function removeEolLast(diagnostic: BsDiagnostic): ChangeEntry {
return {
diagnostic,
changes: [
replaceText(diagnostic.range, '')
]
};
}
3 changes: 2 additions & 1 deletion src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ function rulesToSeverity(rules: BsLintConfig['rules']) {
typeAnnotations: rules['type-annotations'],
noPrint: ruleToSeverity(rules['no-print']),
noTodo: ruleToSeverity(rules['no-todo']),
noStop: ruleToSeverity(rules['no-stop'])
noStop: ruleToSeverity(rules['no-stop']),
eolLast: rules['eol-last']
};
}

Expand Down
Empty file added test/project1/source/empty.brs
Empty file.
3 changes: 3 additions & 0 deletions test/project1/source/eol-last.brs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
sub init()
label = CreateObject("roSGnode", "Label")
end sub
Loading