Merge pull request #54 from sasjs/issue53

Issue53

.git-hooks/commit-msg

@@ -0,0 +1,18 @@
+#!/bin/sh
+RED="\033[1;31m"
+GREEN="\033[1;32m"
+
+# Get the commit message (the parameter we're given is just the path to the
+# temporary file which holds the message).
+commit_message=$(cat "$1")
+
+if (echo "$commit_message" | grep -Eq "^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test)(\([a-z \-]+\))?!?: .+$") then
+   echo "${GREEN} ✔ Commit message meets Conventional Commit standards"
+   exit 0
+fi
+
+echo "${RED}❌ Commit message does not meet the Conventional Commit standard!"
+echo "An example of a valid message is:"
+echo "  feat(login): add the 'remember me' button"
+echo "ℹ More details at: https://www.conventionalcommits.org/en/v1.0.0/#summary"
+exit 1

README.md

@@ -16,7 +16,7 @@ Configuration is via a `.sasjslint` file with the following structure (these are
 {
     "noEncodedPasswords": true,
     "hasDoxygenHeader": true,
-    "hasMacroNameInMend": false,
+    "hasMacroNameInMend": true,
     "hasMacroParentheses": true,
     "indentationMultiple": 2,
     "lowerCaseFileNames": true,
@@ -46,7 +46,7 @@ The SASjs framework recommends the use of Doxygen headers for describing all typ
 #### hasMacroNameInMend
 The addition of the macro name in the `%mend` statement is optional, but can approve readability in large programs.  A discussion on this topic can be found [here](https://www.linkedin.com/posts/allanbowe_sas-sasapps-sasjs-activity-6783413360781266945-1-7m).  The default setting will be the result of a popular vote by around 300 people.
 
-* Default:  false (for now)
+* Default:  true
 * Severity: WARNING
 
 #### hasMacroParentheses

package.json

@@ -8,7 +8,8 @@
     "postpublish": "git clean -fd",
     "package:lib": "npm run build && cp ./package.json build && cp README.md build && cd build && npm version \"5.0.0\" && npm pack",
     "lint:fix": "npx prettier --write '{src,test}/**/*.{ts,tsx,js,jsx,html,css,sass,less,json,yml,md,graphql}'",
-    "lint": "npx prettier --check '{src,test}/**/*.{ts,tsx,js,jsx,html,css,sass,less,json,yml,md,graphql}'"
+    "lint": "npx prettier --check '{src,test}/**/*.{ts,tsx,js,jsx,html,css,sass,less,json,yml,md,graphql}'",
+    "postinstall": "[ -d .git ] && git config core.hooksPath ./.git-hooks || true"
   },
   "publishConfig": {
     "access": "public"

sasjslint-schema.json

@@ -16,7 +16,8 @@
     "noSpacesInFileNames": true,
     "noTabIndentation": true,
     "noTrailingSpaces": true,
-    "lineEndings": "lf"
+    "lineEndings": "lf",
+    "strictMacroDefinition": true
   },
   "examples": [
     {
@@ -31,7 +32,8 @@
       "hasMacroNameInMend": true,
       "noNestedMacros": true,
       "hasMacroParentheses": true,
-      "lineEndings": "crlf"
+      "lineEndings": "crlf",
+      "strictMacroDefinition": true
     }
   ],
   "properties": {
@@ -130,6 +132,14 @@
       "description": "Enforces the configured terminating character for each line. Shows a warning when incorrect line endings are present.",
       "default": "lf",
       "examples": ["lf", "crlf"]
+    },
+    "strictMacroDefinition": {
+      "$id": "#/properties/strictMacroDefinition",
+      "type": "boolean",
+      "title": "strictMacroDefinition",
+      "description": "Enforces Macro Definition syntax. Shows a warning when incorrect syntax is used.",
+      "default": true,
+      "examples": [true, false]
     }
   }
 }

src/rules/file/hasDoxygenHeader.spec.ts

@@ -59,7 +59,7 @@ describe('hasDoxygenHeader - test', () => {
   it('should return an array with a single diagnostic when the file is undefined', () => {
     const content = undefined
 
-    expect(hasDoxygenHeader.test((content as unknown) as string)).toEqual([
+    expect(hasDoxygenHeader.test(content as unknown as string)).toEqual([
       {
         message: 'File missing Doxygen header',
         lineNumber: 1,

src/rules/file/hasMacroNameInMend.spec.ts

@@ -114,7 +114,7 @@ describe('hasMacroNameInMend - test', () => {
   it('should return an empty array when the file is undefined', () => {
     const content = undefined
 
-    expect(hasMacroNameInMend.test((content as unknown) as string)).toEqual([])
+    expect(hasMacroNameInMend.test(content as unknown as string)).toEqual([])
   })
 
   describe('nestedMacros', () => {

src/rules/file/hasMacroParentheses.spec.ts

@@ -79,7 +79,7 @@ describe('hasMacroParentheses', () => {
   it('should return an empty array when the file is undefined', () => {
     const content = undefined
 
-    expect(hasMacroParentheses.test((content as unknown) as string)).toEqual([])
+    expect(hasMacroParentheses.test(content as unknown as string)).toEqual([])
   })
 
   describe('with extra spaces and comments', () => {

src/rules/file/noNestedMacros.spec.ts

@@ -72,7 +72,7 @@ describe('noNestedMacros', () => {
   it('should return an empty array when the file is undefined', () => {
     const content = undefined
 
-    expect(noNestedMacros.test((content as unknown) as string)).toEqual([])
+    expect(noNestedMacros.test(content as unknown as string)).toEqual([])
   })
 
   it('should use the configured line ending while testing content', () => {

src/rules/line/index.ts

@@ -3,3 +3,4 @@ export { maxLineLength } from './maxLineLength'
 export { noEncodedPasswords } from './noEncodedPasswords'
 export { noTabIndentation } from './noTabIndentation'
 export { noTrailingSpaces } from './noTrailingSpaces'
+export { strictMacroDefinition } from './strictMacroDefinition'

src/rules/line/strictMacroDefinition.spec.ts

@@ -0,0 +1,88 @@
+import { LintConfig, Severity } from '../../types'
+import { strictMacroDefinition } from './strictMacroDefinition'
+
+describe('strictMacroDefinition', () => {
+  it('should return an empty array when the line has correct macro definition syntax', () => {
+    const line = '%macro somemacro;'
+    expect(strictMacroDefinition.test(line, 1)).toEqual([])
+
+    const line2 = '%macro somemacro();'
+    expect(strictMacroDefinition.test(line2, 1)).toEqual([])
+
+    const line3 = '%macro somemacro(var1);'
+    expect(strictMacroDefinition.test(line3, 1)).toEqual([])
+
+    const line4 = '%macro somemacro/minoperator;'
+    expect(strictMacroDefinition.test(line4, 1)).toEqual([])
+
+    const line5 = '%macro somemacro /minoperator;'
+    expect(strictMacroDefinition.test(line5, 1)).toEqual([])
+
+    const line6 = '%macro somemacro(var1, var2)/minoperator;'
+    expect(strictMacroDefinition.test(line6, 1)).toEqual([])
+
+    const line7 =
+      ' /* Some Comment */ %macro somemacro(var1, var2) /minoperator ; /* Some Comment */'
+    expect(strictMacroDefinition.test(line7, 1)).toEqual([])
+  })
+
+  it('should return an array with a single diagnostic when Macro definition has space in param', () => {
+    const line = '%macro somemacro(va r1);'
+    expect(strictMacroDefinition.test(line, 1)).toEqual([
+      {
+        message: `Param 'va r1' cannot have space`,
+        lineNumber: 1,
+        startColumnNumber: 18,
+        endColumnNumber: 22,
+        severity: Severity.Warning
+      }
+    ])
+  })
+
+  it('should return an array with a two diagnostics when Macro definition has space in param', () => {
+    const line = '%macro somemacro(var1, var 2, v ar3, var4);'
+    expect(strictMacroDefinition.test(line, 1)).toEqual([
+      {
+        message: `Param 'var 2' cannot have space`,
+        lineNumber: 1,
+        startColumnNumber: 24,
+        endColumnNumber: 28,
+        severity: Severity.Warning
+      },
+      {
+        message: `Param 'v ar3' cannot have space`,
+        lineNumber: 1,
+        startColumnNumber: 31,
+        endColumnNumber: 35,
+        severity: Severity.Warning
+      }
+    ])
+  })
+
+  it('should return an array with a single diagnostic when Macro definition has invalid option', () => {
+    const line = '%macro somemacro(var1, var2)/minXoperator;'
+    expect(strictMacroDefinition.test(line, 1)).toEqual([
+      {
+        message: `Option 'minXoperator' is not valid`,
+        lineNumber: 1,
+        startColumnNumber: 30,
+        endColumnNumber: 41,
+        severity: Severity.Warning
+      }
+    ])
+  })
+
+  it('should return an array with a two diagnostics when Macro definition has invalid options', () => {
+    const line =
+      '%macro somemacro(var1, var2)/ store   invalidoption   secure ;'
+    expect(strictMacroDefinition.test(line, 1)).toEqual([
+      {
+        message: `Option 'invalidoption' is not valid`,
+        lineNumber: 1,
+        startColumnNumber: 39,
+        endColumnNumber: 51,
+        severity: Severity.Warning
+      }
+    ])
+  })
+})

src/rules/line/strictMacroDefinition.ts

@@ -0,0 +1,88 @@
+import { Diagnostic } from '../../types/Diagnostic'
+import { LintConfig } from '../../types'
+import { LineLintRule } from '../../types/LintRule'
+import { LintRuleType } from '../../types/LintRuleType'
+import { Severity } from '../../types/Severity'
+import { parseMacros } from '../../utils/parseMacros'
+
+const name = 'strictMacroDefinition'
+const description = 'Enforce strictly rules of macro definition syntax.'
+const message = 'Incorrent Macro Definition Syntax'
+
+const validOptions = [
+  'CMD',
+  'DES',
+  'MINDELIMITER',
+  'MINOPERATOR',
+  'NOMINOPERATOR',
+  'PARMBUFF',
+  'SECURE',
+  'NOSECURE',
+  'STMT',
+  'SOURCE',
+  'SRC',
+  'STORE'
+]
+
+const test = (value: string, lineNumber: number) => {
+  const diagnostics: Diagnostic[] = []
+
+  const macros = parseMacros(value)
+  const declaration = macros[0]?.declaration
+  if (!declaration) return []
+
+  const regExpParams = new RegExp(/\((.*?)\)/)
+  const regExpParamsResult = regExpParams.exec(declaration)
+
+  let _declaration = declaration
+  if (regExpParamsResult) {
+    const paramsPresent = regExpParamsResult[1]
+
+    const paramsTrimmed = paramsPresent.trim()
+    const params = paramsTrimmed.split(',')
+    params.forEach((param) => {
+      const trimedParam = param.split('=')[0].trim()
+      if (trimedParam.includes(' ')) {
+        diagnostics.push({
+          message: `Param '${trimedParam}' cannot have space`,
+          lineNumber,
+          startColumnNumber: value.indexOf(trimedParam) + 1,
+          endColumnNumber: value.indexOf(trimedParam) + trimedParam.length,
+          severity: Severity.Warning
+        })
+      }
+    })
+
+    _declaration = declaration.split(`(${paramsPresent})`)[1]
+  }
+
+  const optionsPresent = _declaration.split('/')?.[1]?.trim().split(' ')
+
+  optionsPresent
+    ?.filter((o) => !!o)
+    .forEach((option) => {
+      const trimmedOption = option.trim()
+      if (!validOptions.includes(trimmedOption.toUpperCase())) {
+        diagnostics.push({
+          message: `Option '${trimmedOption}' is not valid`,
+          lineNumber,
+          startColumnNumber: value.indexOf(trimmedOption) + 1,
+          endColumnNumber: value.indexOf(trimmedOption) + trimmedOption.length,
+          severity: Severity.Warning
+        })
+      }
+    })
+
+  return diagnostics
+}
+
+/**
+ * Lint rule that checks if a line has followed syntax for macro definition
+ */
+export const strictMacroDefinition: LineLintRule = {
+  type: LintRuleType.Line,
+  name,
+  description,
+  message,
+  test
+}

src/types/LintConfig.ts

@@ -10,7 +10,8 @@ import {
   maxLineLength,
   noEncodedPasswords,
   noTabIndentation,
-  noTrailingSpaces
+  noTrailingSpaces,
+  strictMacroDefinition
 } from '../rules/line'
 import { lowerCaseFileNames, noSpacesInFileNames } from '../rules/path'
 import { LineEndings } from './LineEndings'
@@ -90,5 +91,9 @@ export class LintConfig {
     if (json?.hasMacroParentheses) {
       this.fileLintRules.push(hasMacroParentheses)
     }
+
+    if (json?.strictMacroDefinition) {
+      this.lineLintRules.push(strictMacroDefinition)
+    }
   }
 }

src/utils/getLintConfig.spec.ts

@@ -3,7 +3,7 @@ import { LintConfig } from '../types/LintConfig'
 import { getLintConfig } from './getLintConfig'
 
 const expectedFileLintRulesCount = 4
-const expectedLineLintRulesCount = 5
+const expectedLineLintRulesCount = 6
 const expectedPathLintRulesCount = 2
 
 describe('getLintConfig', () => {

src/utils/getLintConfig.ts

@@ -17,7 +17,8 @@ export const DefaultLintConfiguration = {
   indentationMultiple: 2,
   hasMacroNameInMend: true,
   noNestedMacros: true,
-  hasMacroParentheses: true
+  hasMacroParentheses: true,
+  strictMacroDefinition: true
 }
 
 /**

src/utils/parseMacros.spec.ts

@@ -25,6 +25,76 @@ describe('parseMacros', () => {
     })
   })
 
+  it('should return an array with a single macro having parameters', () => {
+    const text = `%macro test(var,sum);
+  %put 'hello';
+%mend`
+
+    const macros = parseMacros(text, new LintConfig())
+
+    expect(macros.length).toEqual(1)
+    expect(macros).toContainEqual({
+      name: 'test',
+      declarationLine: '%macro test(var,sum);',
+      terminationLine: '%mend',
+      declaration: '%macro test(var,sum)',
+      termination: '%mend',
+      startLineNumber: 1,
+      endLineNumber: 3,
+      parentMacro: '',
+      hasMacroNameInMend: false,
+      hasParentheses: false,
+      mismatchedMendMacroName: ''
+    })
+  })
+
+  it('should return an array with a single macro having PARMBUFF option', () => {
+    const text = `%macro test/parmbuff;
+  %put 'hello';
+%mend`
+
+    const macros = parseMacros(text, new LintConfig())
+
+    expect(macros.length).toEqual(1)
+    expect(macros).toContainEqual({
+      name: 'test',
+      declarationLine: '%macro test/parmbuff;',
+      terminationLine: '%mend',
+      declaration: '%macro test/parmbuff',
+      termination: '%mend',
+      startLineNumber: 1,
+      endLineNumber: 3,
+      parentMacro: '',
+      hasMacroNameInMend: false,
+      hasParentheses: false,
+      mismatchedMendMacroName: ''
+    })
+  })
+
+  it('should return an array with a single macro having paramerter & SOURCE option', () => {
+    const text = `/* commentary */  %macro foobar(arg) /store source
+      des="This macro does not do much";
+  %put 'hello';
+%mend`
+
+    const macros = parseMacros(text, new LintConfig())
+
+    expect(macros.length).toEqual(1)
+    expect(macros).toContainEqual({
+      name: 'foobar',
+      declarationLine: '/* commentary */  %macro foobar(arg) /store source',
+      terminationLine: '%mend',
+      declaration: '%macro foobar(arg) /store source',
+      termination: '%mend',
+      startLineNumber: 1,
+      endLineNumber: 4,
+      parentMacro: '',
+      hasMacroNameInMend: false,
+      hasParentheses: false,
+      mismatchedMendMacroName: ''
+    })
+  })
+
   it('should return an array with multiple macros', () => {
     const text = `%macro foo;
   %put 'foo';

src/utils/parseMacros.ts

@@ -43,6 +43,7 @@ export const parseMacros = (text: string, config?: LintConfig): Macro[] => {
         const name = trimmedStatement
           .slice(7, trimmedStatement.length)
           .trim()
+          .split('/')[0]
           .split('(')[0]
         macroStack.push({
           name,

src/utils/trimComments.ts

@@ -6,11 +6,14 @@ export const trimComments = (
 
   if (commentStarted || trimmed.startsWith('/*')) {
     const parts = trimmed.split('*/')
-    if (parts.length > 1) {
+    if (parts.length === 2) {
       return {
         statement: (parts.pop() as string).trim(),
         commentStarted: false
       }
+    } else if (parts.length > 2) {
+      parts.shift()
+      return trimComments(parts.join('*/'), false)
     } else {
       return { statement: '', commentStarted: true }
     }