diff --git a/.sasjslint b/.sasjslint index 2fef118..2d6a33f 100644 --- a/.sasjslint +++ b/.sasjslint @@ -7,5 +7,7 @@ "lowerCaseFileNames": true, "noTabIndentation": true, "indentationMultiple": 2, - "hasMacroNameInMend": false + "hasMacroNameInMend": false, + "noNestedMacros": true, + "hasMacroParentheses": true } \ No newline at end of file diff --git a/src/rules/hasMacroNameInMend.spec.ts b/src/rules/hasMacroNameInMend.spec.ts index 6ea35ca..5876305 100644 --- a/src/rules/hasMacroNameInMend.spec.ts +++ b/src/rules/hasMacroNameInMend.spec.ts @@ -48,7 +48,7 @@ describe('hasMacroNameInMend', () => { message: 'mismatch macro name in %mend statement', lineNumber: 4, startColumnNumber: 9, - endColumnNumber: 25, + endColumnNumber: 24, severity: Severity.Warning } ]) @@ -219,7 +219,7 @@ describe('hasMacroNameInMend', () => { message: 'mismatch macro name in %mend statement', lineNumber: 6, startColumnNumber: 14, - endColumnNumber: 30, + endColumnNumber: 29, severity: Severity.Warning } ]) diff --git a/src/rules/hasMacroNameInMend.ts b/src/rules/hasMacroNameInMend.ts index 338bf5a..2081f77 100644 --- a/src/rules/hasMacroNameInMend.ts +++ b/src/rules/hasMacroNameInMend.ts @@ -2,6 +2,9 @@ import { Diagnostic } from '../types/Diagnostic' import { FileLintRule } from '../types/LintRule' import { LintRuleType } from '../types/LintRuleType' import { Severity } from '../types/Severity' +import { trimComments } from '../utils/trimComments' +import { getLineNumber } from '../utils/getLineNumber' +import { getColNumber } from '../utils/getColNumber' const name = 'hasMacroNameInMend' const description = 'The %mend statement should contain the macro name' @@ -22,8 +25,8 @@ const test = (value: string) => { if (trimmedStatement.startsWith('%macro ')) { const macroName = trimmedStatement - .split(' ') - .filter((s: string) => !!s)[1] + .slice(7, trimmedStatement.length) + .trim() .split('(')[0] stack.push(macroName) } else if (trimmedStatement.startsWith('%mend')) { @@ -46,7 +49,7 @@ const test = (value: string) => { lineNumber: getLineNumber(statements, index + 1), startColumnNumber: getColNumber(statement, macroName), endColumnNumber: - getColNumber(statement, macroName) + macroName.length, + getColNumber(statement, macroName) + macroName.length - 1, severity: Severity.Warning }) } @@ -64,36 +67,6 @@ const test = (value: string) => { return diagnostics } -const trimComments = ( - statement: string, - commentStarted: boolean = false -): { statement: string; commentStarted: boolean } => { - let trimmed = statement.trim() - - if (commentStarted || trimmed.startsWith('/*')) { - const parts = trimmed.split('*/') - if (parts.length > 1) { - return { - statement: (parts.pop() as string).trim(), - commentStarted: false - } - } else { - return { statement: '', commentStarted: true } - } - } - return { statement: trimmed, commentStarted: false } -} - -const getLineNumber = (statements: string[], index: number): number => { - const combinedCode = statements.slice(0, index).join(';') - const lines = (combinedCode.match(/\n/g) || []).length + 1 - return lines -} - -const getColNumber = (statement: string, text: string): number => { - return (statement.split('\n').pop() as string).indexOf(text) + 1 -} - /** * Lint rule that checks for the presence of macro name in %mend statement. */ diff --git a/src/rules/hasMacroParentheses.spec.ts b/src/rules/hasMacroParentheses.spec.ts new file mode 100644 index 0000000..bb77ae7 --- /dev/null +++ b/src/rules/hasMacroParentheses.spec.ts @@ -0,0 +1,128 @@ +import { Severity } from '../types/Severity' +import { hasMacroParentheses } from './hasMacroParentheses' + +describe('hasMacroParentheses', () => { + it('should return an empty array when macro defined correctly', () => { + const content = ` + %macro somemacro(); + %put &sysmacroname; + %mend somemacro;` + + expect(hasMacroParentheses.test(content)).toEqual([]) + }) + + it('should return an array with a single diagnostics when macro defined without parentheses', () => { + const content = ` + %macro somemacro; + %put &sysmacroname; + %mend somemacro;` + + expect(hasMacroParentheses.test(content)).toEqual([ + { + message: 'Macro definition missing parentheses', + lineNumber: 2, + startColumnNumber: 10, + endColumnNumber: 18, + severity: Severity.Warning + } + ]) + }) + + it('should return an array with a single diagnostics when macro defined without name', () => { + const content = ` + %macro (); + %put &sysmacroname; + %mend;` + + expect(hasMacroParentheses.test(content)).toEqual([ + { + message: 'Macro definition missing name', + lineNumber: 2, + startColumnNumber: 3, + endColumnNumber: 12, + severity: Severity.Warning + } + ]) + }) + + it('should return an array with a single diagnostics when macro defined without name and parentheses', () => { + const content = ` + %macro ; + %put &sysmacroname; + %mend;` + + expect(hasMacroParentheses.test(content)).toEqual([ + { + message: 'Macro definition missing name', + lineNumber: 2, + startColumnNumber: 3, + endColumnNumber: 10, + severity: Severity.Warning + } + ]) + }) + + it('should return an empty array when the file is undefined', () => { + const content = undefined + + expect(hasMacroParentheses.test((content as unknown) as string)).toEqual([]) + }) + + describe('with extra spaces and comments', () => { + it('should return an empty array when %mend has correct macro name', () => { + const content = ` + /* 1st comment */ + %macro somemacro(); + + %put &sysmacroname; + + /* 2nd + comment */ + /* 3rd comment */ %mend somemacro ;` + + expect(hasMacroParentheses.test(content)).toEqual([]) + }) + + it('should return an array with a single diagnostic when macro defined without parentheses having code in comments', () => { + const content = `/** + @file examplemacro.sas + @brief an example of a macro to be used in a service + @details This macro is great. Yadda yadda yadda. Usage: + + * code formatting applies when indented by 4 spaces; code formatting applies when indented by 4 spaces; code formatting applies when indented by 4 spaces; code formatting applies when indented by 4 spaces; code formatting applies when indented by 4 spaces; + + some code + %macro examplemacro123(); + + %examplemacro() + +

SAS Macros

+ @li doesnothing.sas + + @author Allan Bowe + **/ + + %macro examplemacro; + + proc sql; + create table areas + as select area + + from sashelp.springs; + + %doesnothing(); + + %mend;` + + expect(hasMacroParentheses.test(content)).toEqual([ + { + message: 'Macro definition missing parentheses', + lineNumber: 19, + startColumnNumber: 12, + endColumnNumber: 23, + severity: Severity.Warning + } + ]) + }) + }) +}) diff --git a/src/rules/hasMacroParentheses.ts b/src/rules/hasMacroParentheses.ts new file mode 100644 index 0000000..fb894fa --- /dev/null +++ b/src/rules/hasMacroParentheses.ts @@ -0,0 +1,77 @@ +import { Diagnostic } from '../types/Diagnostic' +import { FileLintRule } from '../types/LintRule' +import { LintRuleType } from '../types/LintRuleType' +import { Severity } from '../types/Severity' +import { trimComments } from '../utils/trimComments' +import { getLineNumber } from '../utils/getLineNumber' +import { getColNumber } from '../utils/getColNumber' + +const name = 'hasMacroParentheses' +const description = 'Macros are always defined with parentheses' +const message = 'Macro definition missing parentheses' +const test = (value: string) => { + const diagnostics: Diagnostic[] = [] + + const statements: string[] = value ? value.split(';') : [] + + let trimmedStatement = '', + commentStarted = false + statements.forEach((statement, index) => { + ;({ statement: trimmedStatement, commentStarted } = trimComments( + statement, + commentStarted + )) + + if (trimmedStatement.startsWith('%macro')) { + const macroNameDefinition = trimmedStatement + .slice(7, trimmedStatement.length) + .trim() + + const macroNameDefinitionParts = macroNameDefinition.split('(') + const macroName = macroNameDefinitionParts[0] + + if (!macroName) + diagnostics.push({ + message: 'Macro definition missing name', + lineNumber: getLineNumber(statements, index + 1), + startColumnNumber: getColNumber(statement, '%macro'), + endColumnNumber: statement.length, + severity: Severity.Warning + }) + else if (macroNameDefinitionParts.length === 1) + diagnostics.push({ + message, + lineNumber: getLineNumber(statements, index + 1), + startColumnNumber: getColNumber(statement, macroNameDefinition), + endColumnNumber: + getColNumber(statement, macroNameDefinition) + + macroNameDefinition.length - + 1, + severity: Severity.Warning + }) + else if (macroName !== macroName.trim()) + diagnostics.push({ + message: 'Macro definition cannot have space', + lineNumber: getLineNumber(statements, index + 1), + startColumnNumber: getColNumber(statement, macroNameDefinition), + endColumnNumber: + getColNumber(statement, macroNameDefinition) + + macroNameDefinition.length - + 1, + severity: Severity.Warning + }) + } + }) + return diagnostics +} + +/** + * Lint rule that checks for the presence of macro name in %mend statement. + */ +export const hasMacroParentheses: FileLintRule = { + type: LintRuleType.File, + name, + description, + message, + test +} diff --git a/src/rules/noNestedMacros.spec.ts b/src/rules/noNestedMacros.spec.ts new file mode 100644 index 0000000..a965190 --- /dev/null +++ b/src/rules/noNestedMacros.spec.ts @@ -0,0 +1,78 @@ +import { Severity } from '../types/Severity' +import { noNestedMacros } from './noNestedMacros' + +describe('noNestedMacros', () => { + it('should return an empty array when no nested macro', () => { + const content = ` + %macro somemacro(); + %put &sysmacroname; + %mend somemacro;` + + expect(noNestedMacros.test(content)).toEqual([]) + }) + + it('should return an array with a single diagnostics when nested macro defined', () => { + const content = ` + %macro outer(); + /* any amount of arbitrary code */ + %macro inner(); + %put inner; + %mend; + %inner() + %put outer; + %mend; + + %outer()` + + expect(noNestedMacros.test(content)).toEqual([ + { + message: "Macro definition present inside another macro 'outer'", + lineNumber: 4, + startColumnNumber: 7, + endColumnNumber: 20, + severity: Severity.Warning + } + ]) + }) + + it('should return an array with a single diagnostics when nested macro defined 2 levels', () => { + const content = ` + %macro outer(); + /* any amount of arbitrary code */ + %macro inner(); + %put inner; + + %macro inner2(); + %put inner2; + %mend; + %mend; + %inner() + %put outer; + %mend; + + %outer()` + + expect(noNestedMacros.test(content)).toEqual([ + { + message: "Macro definition present inside another macro 'outer'", + lineNumber: 4, + startColumnNumber: 7, + endColumnNumber: 20, + severity: Severity.Warning + }, + { + message: "Macro definition present inside another macro 'inner'", + lineNumber: 7, + startColumnNumber: 17, + endColumnNumber: 31, + severity: Severity.Warning + } + ]) + }) + + it('should return an empty array when the file is undefined', () => { + const content = undefined + + expect(noNestedMacros.test((content as unknown) as string)).toEqual([]) + }) +}) diff --git a/src/rules/noNestedMacros.ts b/src/rules/noNestedMacros.ts new file mode 100644 index 0000000..2fb5fc9 --- /dev/null +++ b/src/rules/noNestedMacros.ts @@ -0,0 +1,59 @@ +import { Diagnostic } from '../types/Diagnostic' +import { FileLintRule } from '../types/LintRule' +import { LintRuleType } from '../types/LintRuleType' +import { Severity } from '../types/Severity' +import { trimComments } from '../utils/trimComments' +import { getLineNumber } from '../utils/getLineNumber' +import { getColNumber } from '../utils/getColNumber' + +const name = 'noNestedMacros' +const description = 'Defining nested macro is not good practice' +const message = 'Macro definition present inside another macro' +const test = (value: string) => { + const diagnostics: Diagnostic[] = [] + + const statements: string[] = value ? value.split(';') : [] + + const stack: string[] = [] + let trimmedStatement = '', + commentStarted = false + statements.forEach((statement, index) => { + ;({ statement: trimmedStatement, commentStarted } = trimComments( + statement, + commentStarted + )) + + if (trimmedStatement.startsWith('%macro ')) { + const macroName = trimmedStatement + .slice(7, trimmedStatement.length) + .trim() + .split('(')[0] + if (stack.length) { + const parentMacro = stack.slice(-1).pop() + diagnostics.push({ + message: `${message} '${parentMacro}'`, + lineNumber: getLineNumber(statements, index + 1), + startColumnNumber: getColNumber(statement, '%macro'), + endColumnNumber: + getColNumber(statement, '%macro') + trimmedStatement.length - 1, + severity: Severity.Warning + }) + } + stack.push(macroName) + } else if (trimmedStatement.startsWith('%mend')) { + stack.pop() + } + }) + return diagnostics +} + +/** + * Lint rule that checks for the presence of macro name in %mend statement. + */ +export const noNestedMacros: FileLintRule = { + type: LintRuleType.File, + name, + description, + message, + test +} diff --git a/src/types/LintConfig.spec.ts b/src/types/LintConfig.spec.ts index bc88fc3..3ea3bb5 100644 --- a/src/types/LintConfig.spec.ts +++ b/src/types/LintConfig.spec.ts @@ -58,6 +58,42 @@ describe('LintConfig', () => { expect(config.fileLintRules.length).toEqual(0) }) + it('should create an instance with the noNestedMacros flag set', () => { + const config = new LintConfig({ noNestedMacros: true }) + + expect(config).toBeTruthy() + expect(config.lineLintRules.length).toEqual(0) + expect(config.fileLintRules.length).toEqual(1) + expect(config.fileLintRules[0].name).toEqual('noNestedMacros') + expect(config.fileLintRules[0].type).toEqual(LintRuleType.File) + }) + + it('should create an instance with the noNestedMacros flag off', () => { + const config = new LintConfig({ noNestedMacros: false }) + + expect(config).toBeTruthy() + expect(config.lineLintRules.length).toEqual(0) + expect(config.fileLintRules.length).toEqual(0) + }) + + it('should create an instance with the hasMacroParentheses flag set', () => { + const config = new LintConfig({ hasMacroParentheses: true }) + + expect(config).toBeTruthy() + expect(config.lineLintRules.length).toEqual(0) + expect(config.fileLintRules.length).toEqual(1) + expect(config.fileLintRules[0].name).toEqual('hasMacroParentheses') + expect(config.fileLintRules[0].type).toEqual(LintRuleType.File) + }) + + it('should create an instance with the hasMacroParentheses flag off', () => { + const config = new LintConfig({ hasMacroParentheses: false }) + + expect(config).toBeTruthy() + expect(config.lineLintRules.length).toEqual(0) + expect(config.fileLintRules.length).toEqual(0) + }) + it('should create an instance with the indentation multiple set', () => { const config = new LintConfig({ indentationMultiple: 5 }) @@ -82,7 +118,9 @@ describe('LintConfig', () => { maxLineLength: 80, noTabIndentation: true, indentationMultiple: 2, - hasMacroNameInMend: true + hasMacroNameInMend: true, + noNestedMacros: true, + hasMacroParentheses: true }) expect(config).toBeTruthy() @@ -98,11 +136,15 @@ describe('LintConfig', () => { expect(config.lineLintRules[4].name).toEqual('indentationMultiple') expect(config.lineLintRules[4].type).toEqual(LintRuleType.Line) - expect(config.fileLintRules.length).toEqual(2) + expect(config.fileLintRules.length).toEqual(4) expect(config.fileLintRules[0].name).toEqual('hasDoxygenHeader') expect(config.fileLintRules[0].type).toEqual(LintRuleType.File) expect(config.fileLintRules[1].name).toEqual('hasMacroNameInMend') expect(config.fileLintRules[1].type).toEqual(LintRuleType.File) + expect(config.fileLintRules[2].name).toEqual('noNestedMacros') + expect(config.fileLintRules[2].type).toEqual(LintRuleType.File) + expect(config.fileLintRules[3].name).toEqual('hasMacroParentheses') + expect(config.fileLintRules[3].type).toEqual(LintRuleType.File) expect(config.pathLintRules.length).toEqual(2) expect(config.pathLintRules[0].name).toEqual('noSpacesInFileNames') diff --git a/src/types/LintConfig.ts b/src/types/LintConfig.ts index f8e57c2..bec64f5 100644 --- a/src/types/LintConfig.ts +++ b/src/types/LintConfig.ts @@ -7,6 +7,8 @@ import { noSpacesInFileNames } from '../rules/noSpacesInFileNames' import { noTabIndentation } from '../rules/noTabIndentation' import { noTrailingSpaces } from '../rules/noTrailingSpaces' import { hasMacroNameInMend } from '../rules/hasMacroNameInMend' +import { noNestedMacros } from '../rules/noNestedMacros' +import { hasMacroParentheses } from '../rules/hasMacroParentheses' import { FileLintRule, LineLintRule, PathLintRule } from './LintRule' /** @@ -61,5 +63,13 @@ export class LintConfig { if (json?.hasMacroNameInMend) { this.fileLintRules.push(hasMacroNameInMend) } + + if (json?.noNestedMacros) { + this.fileLintRules.push(noNestedMacros) + } + + if (json?.hasMacroParentheses) { + this.fileLintRules.push(hasMacroParentheses) + } } } diff --git a/src/utils/getColNumber.ts b/src/utils/getColNumber.ts new file mode 100644 index 0000000..4333bd6 --- /dev/null +++ b/src/utils/getColNumber.ts @@ -0,0 +1,3 @@ +export const getColNumber = (statement: string, text: string): number => { + return (statement.split('\n').pop() as string).indexOf(text) + 1 +} diff --git a/src/utils/getLineNumber.ts b/src/utils/getLineNumber.ts new file mode 100644 index 0000000..ebfe055 --- /dev/null +++ b/src/utils/getLineNumber.ts @@ -0,0 +1,5 @@ +export const getLineNumber = (statements: string[], index: number): number => { + const combinedCode = statements.slice(0, index).join(';') + const lines = (combinedCode.match(/\n/g) || []).length + 1 + return lines +} diff --git a/src/utils/getLintConfig.spec.ts b/src/utils/getLintConfig.spec.ts index e7e4870..1ea0d94 100644 --- a/src/utils/getLintConfig.spec.ts +++ b/src/utils/getLintConfig.spec.ts @@ -17,7 +17,7 @@ describe('getLintConfig', () => { const config = await getLintConfig() expect(config).toBeInstanceOf(LintConfig) - expect(config.fileLintRules.length).toEqual(1) + expect(config.fileLintRules.length).toEqual(3) expect(config.lineLintRules.length).toEqual(5) expect(config.pathLintRules.length).toEqual(2) }) diff --git a/src/utils/getLintConfig.ts b/src/utils/getLintConfig.ts index 9cd359d..43df7c4 100644 --- a/src/utils/getLintConfig.ts +++ b/src/utils/getLintConfig.ts @@ -15,7 +15,9 @@ export const DefaultLintConfiguration = { maxLineLength: 80, noTabIndentation: true, indentationMultiple: 2, - hasMacroNameInMend: false + hasMacroNameInMend: false, + noNestedMacros: true, + hasMacroParentheses: true } /** diff --git a/src/utils/trimComments.ts b/src/utils/trimComments.ts new file mode 100644 index 0000000..ad11691 --- /dev/null +++ b/src/utils/trimComments.ts @@ -0,0 +1,19 @@ +export const trimComments = ( + statement: string, + commentStarted: boolean = false +): { statement: string; commentStarted: boolean } => { + let trimmed = statement.trim() + + if (commentStarted || trimmed.startsWith('/*')) { + const parts = trimmed.split('*/') + if (parts.length > 1) { + return { + statement: (parts.pop() as string).trim(), + commentStarted: false + } + } else { + return { statement: '', commentStarted: true } + } + } + return { statement: trimmed, commentStarted: false } +}