Skip to content

Commit

Permalink
Merge pull request #199 from sasjs/issue-45
Browse files Browse the repository at this point in the history
feat: add a new config maxHeaderLineLength
  • Loading branch information
allanbowe authored Jan 11, 2023
2 parents 4cb2fe8 + fa07a77 commit 985ed41
Show file tree
Hide file tree
Showing 13 changed files with 1,890 additions and 10,399 deletions.
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Configuration is via a `.sasjslint` file with the following structure (these are
"ignoreList": ["sajsbuild/", "sasjsresults/"],
"indentationMultiple": 2,
"lowerCaseFileNames": true,
"maxHeaderLineLength": 80,
"maxLineLength": 80,
"noNestedMacros": true,
"noGremlins": true,
Expand Down Expand Up @@ -127,6 +128,20 @@ On *nix systems, it is imperative that autocall macros are in lowercase. When sh
- Default: true
- Severity: WARNING

### maxHeaderLineLength

In a program header it can be necessary to insert items such as URLs or markdown tables, that cannot be split over multiple lines. To avoid the need to increase `maxLineLength` for the entire project, it is possible to raise the line length limit for the header section only.

The `maxHeaderLineLength` setting is always the _higher_ of `maxHeaderLineLength` and `maxLineLength` (if you set a lower number, it is ignored).

- Default: 80
- Severity: WARNING

See also:

* [hasDoxygenHeader](#hasdoxygenheader)
* [maxLineLength](#maxlinelength)

### maxLineLength

Code becomes far more readable when line lengths are short. The most compelling reason for short line lengths is to avoid the need to scroll when performing a side-by-side 'compare' between two files (eg as part of a GIT feature branch review). A longer discussion on optimal code line length can be found [here](https://stackoverflow.com/questions/578059/studies-on-optimal-code-width)
Expand All @@ -138,6 +153,10 @@ We strongly recommend a line length limit, and set the bar at 80. To turn this f
- Default: 80
- Severity: WARNING

See also:

* [maxHeaderLineLength](#maxheaderlinelength)

### noGremlins

Capture zero-width whitespace and other non-standard characters. The logic is borrowed from the [VSCode Gremlins Extension](https://github.com/nhoizey/vscode-gremlins) - if you are looking for more advanced gremlin zapping capabilities, we highly recommend to use their extension instead.
Expand Down
12,132 changes: 1,749 additions & 10,383 deletions package-lock.json

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,16 @@
},
"homepage": "https://github.com/sasjs/lint#readme",
"devDependencies": {
"@types/jest": "^26.0.23",
"@types/node": "^15.12.2",
"all-contributors-cli": "^6.20.0",
"jest": "^26.6.3",
"@types/jest": "29.2.5",
"@types/node": "18.11.18",
"all-contributors-cli": "6.24.0",
"jest": "29.3.1",
"rimraf": "^3.0.2",
"ts-jest": "^26.5.6",
"ts-jest": "29.0.3",
"typescript": "^4.3.2"
},
"dependencies": {
"@sasjs/utils": "^2.19.0",
"ignore": "^5.2.0"
"@sasjs/utils": "2.52.0",
"ignore": "5.2.4"
}
}
10 changes: 10 additions & 0 deletions sasjslint-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"indentationMultiple": 2,
"lowerCaseFileNames": true,
"maxLineLength": 80,
"maxHeaderLineLength": 80,
"noGremlins": true,
"noNestedMacros": true,
"noSpacesInFileNames": true,
Expand All @@ -30,6 +31,7 @@
"noSpacesInFileNames": true,
"lowerCaseFileNames": true,
"maxLineLength": 80,
"maxHeaderLineLength": 80,
"noGremlins": true,
"allowedGremlins": ["0x0080", "0x3000"],
"noTabs": true,
Expand Down Expand Up @@ -127,6 +129,14 @@
"default": 80,
"examples": [60, 80, 120]
},
"maxHeaderLineLength": {
"$id": "#/properties/maxHeaderLineLength",
"type": "number",
"title": "maxHeaderLineLength",
"description": "Enforces a configurable maximum line length for header section. Shows a warning for lines exceeding this length.",
"default": 80,
"examples": [60, 80, 120]
},
"noNestedMacros": {
"$id": "#/properties/noNestedMacros",
"type": "boolean",
Expand Down
13 changes: 9 additions & 4 deletions src/lint/shared.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { LintConfig, Diagnostic } from '../types'
import { splitText } from '../utils'
import { getHeaderLinesCount, splitText } from '../utils'

export const processText = (text: string, config: LintConfig) => {
const lines = splitText(text, config)
const headerLinesCount = getHeaderLinesCount(text, config)
const diagnostics: Diagnostic[] = []
diagnostics.push(...processContent(config, text))
lines.forEach((line, index) => {
diagnostics.push(...processLine(config, line, index + 1))
index += 1
diagnostics.push(
...processLine(config, line, index, index <= headerLinesCount)
)
})

return diagnostics
Expand Down Expand Up @@ -36,11 +40,12 @@ const processContent = (config: LintConfig, content: string): Diagnostic[] => {
export const processLine = (
config: LintConfig,
line: string,
lineNumber: number
lineNumber: number,
isHeaderLine: boolean
): Diagnostic[] => {
const diagnostics: Diagnostic[] = []
config.lineLintRules.forEach((rule) => {
diagnostics.push(...rule.test(line, lineNumber, config))
diagnostics.push(...rule.test(line, lineNumber, config, isHeaderLine))
})

return diagnostics
Expand Down
17 changes: 15 additions & 2 deletions src/rules/line/maxLineLength.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,27 @@ import { LintConfig } from '../../types'
import { LineLintRule } from '../../types/LintRule'
import { LintRuleType } from '../../types/LintRuleType'
import { Severity } from '../../types/Severity'
import { DefaultLintConfiguration } from '../../utils'

const name = 'maxLineLength'
const description = 'Restrict lines to the specified length.'
const message = 'Line exceeds maximum length'

const test = (value: string, lineNumber: number, config?: LintConfig) => {
const test = (
value: string,
lineNumber: number,
config?: LintConfig,
isHeaderLine?: boolean
) => {
const severity = config?.severityLevel[name] || Severity.Warning
const maxLineLength = config?.maxLineLength || 80
let maxLineLength = config
? config.maxLineLength
: DefaultLintConfiguration.maxLineLength

if (isHeaderLine && config) {
maxLineLength = Math.max(config.maxLineLength, config.maxHeaderLineLength)
}

if (value.length <= maxLineLength) return []
return [
{
Expand Down
22 changes: 22 additions & 0 deletions src/types/LintConfig.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,28 @@ describe('LintConfig', () => {
).toBeUndefined()
})

it('should create an instance with the maxLineLength flag off by setting value to 0', () => {
const config = new LintConfig({ maxLineLength: 0 })

expect(config).toBeTruthy()
expect(config.lineLintRules.length).toBeGreaterThan(0)
expect(config.fileLintRules.length).toBeGreaterThan(0)
expect(
config.lineLintRules.find((rule) => rule.name === 'maxLineLength')
).toBeUndefined()
})

it('should create an instance with the maxLineLength flag off by setting value to a negative number', () => {
const config = new LintConfig({ maxLineLength: -1 })

expect(config).toBeTruthy()
expect(config.lineLintRules.length).toBeGreaterThan(0)
expect(config.fileLintRules.length).toBeGreaterThan(0)
expect(
config.lineLintRules.find((rule) => rule.name === 'maxLineLength')
).toBeUndefined()
})

it('should create an instance with the hasDoxygenHeader flag off', () => {
const config = new LintConfig({ hasDoxygenHeader: false })

Expand Down
9 changes: 7 additions & 2 deletions src/types/LintConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export class LintConfig {
readonly fileLintRules: FileLintRule[] = []
readonly pathLintRules: PathLintRule[] = []
readonly maxLineLength: number = 80
readonly maxHeaderLineLength: number = 80
readonly indentationMultiple: number = 2
readonly lineEndings: LineEndings = LineEndings.LF
readonly defaultHeader: string = getDefaultHeader()
Expand Down Expand Up @@ -67,9 +68,13 @@ export class LintConfig {
this.lineLintRules.pop()
}

this.lineLintRules.push(maxLineLength)
if (!isNaN(json?.maxLineLength)) {
if (json?.maxLineLength > 0) {
this.lineLintRules.push(maxLineLength)
this.maxLineLength = json.maxLineLength

if (!isNaN(json?.maxHeaderLineLength)) {
this.maxHeaderLineLength = json.maxHeaderLineLength
}
}

this.fileLintRules.push(lineEndings)
Expand Down
7 changes: 6 additions & 1 deletion src/types/LintRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ export interface LintRule {
*/
export interface LineLintRule extends LintRule {
type: LintRuleType.Line
test: (value: string, lineNumber: number, config?: LintConfig) => Diagnostic[]
test: (
value: string,
lineNumber: number,
config?: LintConfig,
isHeaderLine?: boolean
) => Diagnostic[]
fix?: (value: string, config?: LintConfig) => string
}

Expand Down
21 changes: 21 additions & 0 deletions src/utils/getHeaderLinesCount.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { LintConfig } from '../types'
import { getHeaderLinesCount } from './getHeaderLinesCount'
import { DefaultLintConfiguration } from './getLintConfig'

const sasCodeWithHeader = `/**
@file
@brief <Your brief here>
<h4> SAS Macros </h4>
**/
%put hello world;
`

const sasCodeWithoutHeader = `%put hello world;`

describe('getHeaderLinesCount', () => {
it('should return the number of line header spans upon', () => {
const config = new LintConfig(DefaultLintConfiguration)
expect(getHeaderLinesCount(sasCodeWithHeader, config)).toEqual(5)
expect(getHeaderLinesCount(sasCodeWithoutHeader, config)).toEqual(0)
})
})
23 changes: 23 additions & 0 deletions src/utils/getHeaderLinesCount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { LintConfig } from '../types'
import { splitText } from './splitText'

/**
* This function returns the number of lines the header spans upon.
* The file must start with "/*" and the header will finish with ⇙
*/
export const getHeaderLinesCount = (text: string, config: LintConfig) => {
let count = 0

if (text.trimStart().startsWith('/*')) {
const lines = splitText(text, config)

for (const line of lines) {
count++
if (line.match(/\*\//)) {
break
}
}
}

return count
}
1 change: 1 addition & 0 deletions src/utils/getLintConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const DefaultLintConfiguration = {
noSpacesInFileNames: true,
lowerCaseFileNames: true,
maxLineLength: 80,
maxHeaderLineLength: 80,
noTabIndentation: true,
indentationMultiple: 2,
hasMacroNameInMend: true,
Expand Down
1 change: 1 addition & 0 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export * from './isIgnored'
export * from './listSasFiles'
export * from './splitText'
export * from './getIndicesOf'
export * from './getHeaderLinesCount'

0 comments on commit 985ed41

Please sign in to comment.