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

feat: add new config maxDataLineLength #200

Merged
merged 3 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 2 additions & 2 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Install Dependencies
run: npm ci
- name: Check Code Style
run: npm run lint
- name: Build Project
run: npm run build
- name: Semantic Release
uses: cycjimmy/semantic-release-action@v2
uses: cycjimmy/semantic-release-action@v3
env:
GITHUB_TOKEN: ${{ secrets.GH_TOKEN }}
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
27 changes: 27 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,
"maxDataLineLength": 80,
"maxHeaderLineLength": 80,
"maxLineLength": 80,
"noNestedMacros": true,
Expand Down Expand Up @@ -128,6 +129,30 @@ On *nix systems, it is imperative that autocall macros are in lowercase. When sh
- Default: true
- Severity: WARNING

### maxDataLineLength

Datalines can be very wide, so to avoid the need to increase `maxLineLength` for the entire project, it is possible to raise the line length limit for the data records only. On a related note, as a developer, you should also be aware that code submitted in batch may have a default line length limit which is lower than you expect. See this [usage note](https://support.sas.com/kb/15/883.html) (and thanks to [sasutils for reminding us](https://github.com/sasjs/lint/issues/47#issuecomment-1064340104)).

This feature will work for the following statements:

* cards
* cards4
* datalines
* datalines4
* parmcards
* parmcards4

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

- Default: 80
- Severity: WARNING

See also:

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

### 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.
Expand All @@ -140,6 +165,7 @@ The `maxHeaderLineLength` setting is always the _higher_ of `maxHeaderLineLength
See also:

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

### maxLineLength
Expand All @@ -155,6 +181,7 @@ We strongly recommend a line length limit, and set the bar at 80. To turn this f

See also:

* [maxDataLineLength](#maxdatalinelength)
* [maxHeaderLineLength](#maxheaderlinelength)

### noGremlins
Expand Down
10 changes: 10 additions & 0 deletions sasjslint-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"lowerCaseFileNames": true,
"maxLineLength": 80,
"maxHeaderLineLength": 80,
"maxDataLineLength": 80,
"noGremlins": true,
"noNestedMacros": true,
"noSpacesInFileNames": true,
Expand All @@ -32,6 +33,7 @@
"lowerCaseFileNames": true,
"maxLineLength": 80,
"maxHeaderLineLength": 80,
"maxDataLineLength": 80,
"noGremlins": true,
"allowedGremlins": ["0x0080", "0x3000"],
"noTabs": true,
Expand Down Expand Up @@ -137,6 +139,14 @@
"default": 80,
"examples": [60, 80, 120]
},
"maxDataLineLength": {
"$id": "#/properties/maxDataLineLength",
"type": "number",
"title": "maxDataLineLength",
"description": "Enforces a configurable maximum line length for data 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: 8 additions & 5 deletions src/lint/shared.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import { LintConfig, Diagnostic } from '../types'
import { LintConfig, Diagnostic, LineLintRuleOptions } from '../types'
import { getHeaderLinesCount, splitText } from '../utils'
import { checkIsDataLine, getDataSectionsDetail } from '../utils'

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

Expand Down Expand Up @@ -41,11 +44,11 @@ export const processLine = (
config: LintConfig,
line: string,
lineNumber: number,
isHeaderLine: boolean
options: LineLintRuleOptions
): Diagnostic[] => {
const diagnostics: Diagnostic[] = []
config.lineLintRules.forEach((rule) => {
diagnostics.push(...rule.test(line, lineNumber, config, isHeaderLine))
diagnostics.push(...rule.test(line, lineNumber, config, options))
})

return diagnostics
Expand Down
40 changes: 40 additions & 0 deletions src/rules/line/maxLineLength.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,44 @@ describe('maxLineLength', () => {
'Prow scuttle parrel provost Sail ho shrouds spirits boom mizzenmast yard'
expect(maxLineLength.test(line, 1)).toEqual([])
})

it('should return an array with a single diagnostic when the line in header section exceeds the specified length', () => {
const line = 'This line is from header section'
const config = new LintConfig({
maxLineLength: 10,
maxHeaderLineLength: 15
})
expect(maxLineLength.test(line, 1, config, { isHeaderLine: true })).toEqual(
[
{
message: `Line exceeds maximum length by ${
line.length - config.maxHeaderLineLength
} characters`,
lineNumber: 1,
startColumnNumber: 1,
endColumnNumber: 1,
severity: Severity.Warning
}
]
)
})

it('should return an array with a single diagnostic when the line in data section exceeds the specified length', () => {
const line = 'GROUP_LOGIC:$3. SUBGROUP_LOGIC:$3. SUBGROUP_ID:8.'
const config = new LintConfig({
maxLineLength: 10,
maxDataLineLength: 15
})
expect(maxLineLength.test(line, 1, config, { isDataLine: true })).toEqual([
{
message: `Line exceeds maximum length by ${
line.length - config.maxDataLineLength
} characters`,
lineNumber: 1,
startColumnNumber: 1,
endColumnNumber: 1,
severity: Severity.Warning
}
])
})
})
18 changes: 11 additions & 7 deletions src/rules/line/maxLineLength.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LintConfig } from '../../types'
import { LineLintRule } from '../../types/LintRule'
import { LineLintRule, LineLintRuleOptions } from '../../types/LintRule'
import { LintRuleType } from '../../types/LintRuleType'
import { Severity } from '../../types/Severity'
import { DefaultLintConfiguration } from '../../utils'
Expand All @@ -12,15 +12,19 @@ const test = (
value: string,
lineNumber: number,
config?: LintConfig,
isHeaderLine?: boolean
options?: LineLintRuleOptions
) => {
const severity = config?.severityLevel[name] || Severity.Warning
let maxLineLength = config
? config.maxLineLength
: DefaultLintConfiguration.maxLineLength
let maxLineLength = DefaultLintConfiguration.maxLineLength

if (isHeaderLine && config) {
maxLineLength = Math.max(config.maxLineLength, config.maxHeaderLineLength)
if (config) {
if (options?.isHeaderLine) {
maxLineLength = Math.max(config.maxLineLength, config.maxHeaderLineLength)
} else if (options?.isDataLine) {
maxLineLength = Math.max(config.maxLineLength, config.maxDataLineLength)
} else {
maxLineLength = config.maxLineLength
}
}

if (value.length <= maxLineLength) return []
Expand Down
5 changes: 5 additions & 0 deletions src/types/LintConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class LintConfig {
readonly pathLintRules: PathLintRule[] = []
readonly maxLineLength: number = 80
readonly maxHeaderLineLength: number = 80
readonly maxDataLineLength: number = 80
readonly indentationMultiple: number = 2
readonly lineEndings: LineEndings = LineEndings.LF
readonly defaultHeader: string = getDefaultHeader()
Expand Down Expand Up @@ -75,6 +76,10 @@ export class LintConfig {
if (!isNaN(json?.maxHeaderLineLength)) {
this.maxHeaderLineLength = json.maxHeaderLineLength
}

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

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 @@ -13,6 +13,11 @@ export interface LintRule {
message: string
}

export interface LineLintRuleOptions {
isHeaderLine?: boolean
isDataLine?: boolean
}

/**
* A LineLintRule is run once per line of text.
*/
Expand All @@ -22,7 +27,7 @@ export interface LineLintRule extends LintRule {
value: string,
lineNumber: number,
config?: LintConfig,
isHeaderLine?: boolean
options?: LineLintRuleOptions
) => Diagnostic[]
fix?: (value: string, config?: LintConfig) => string
}
Expand Down
113 changes: 113 additions & 0 deletions src/utils/getDataSectionDetail.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { LintConfig } from '../types'
import { getDataSectionsDetail, checkIsDataLine } from './getDataSectionsDetail'
import { DefaultLintConfiguration } from './getLintConfig'

const datalines = `GROUP_LOGIC:$3. SUBGROUP_LOGIC:$3. SUBGROUP_ID:8. VARIABLE_NM:$32. OPERATOR_NM:$10. RAW_VALUE:$4000.
AND,AND,1,LIBREF,CONTAINS,"'DC'"
AND,OR,2,DSN,=,"'MPE_LOCK_ANYTABLE'"`

const datalinesBeginPattern1 = `datalines;`
const datalinesBeginPattern2 = `datalines4;`
const datalinesBeginPattern3 = `cards;`
const datalinesBeginPattern4 = `cards4;`
const datalinesBeginPattern5 = `parmcards;`
const datalinesBeginPattern6 = `parmcards4;`

const datalinesEndPattern1 = `;`
const datalinesEndPattern2 = `;;;;`

describe('getDataSectionsDetail', () => {
const config = new LintConfig(DefaultLintConfiguration)
it(`should return the detail of data section when it begins and ends with '${datalinesBeginPattern1}' and '${datalinesEndPattern1}' markers`, () => {
const text = `%put hello\n${datalinesBeginPattern1}\n${datalines}\n${datalinesEndPattern1}\n%put world;`
expect(getDataSectionsDetail(text, config)).toEqual([
{
start: 1,
end: 5
}
])
})

it(`should return the detail of data section when it begins and ends with '${datalinesBeginPattern2}' and '${datalinesEndPattern2}' markers`, () => {
const text = `%put hello\n${datalinesBeginPattern2}\n${datalines}\n${datalinesEndPattern2}\n%put world;`
expect(getDataSectionsDetail(text, config)).toEqual([
{
start: 1,
end: 5
}
])
})

it(`should return the detail of data section when it begins and ends with '${datalinesBeginPattern3}' and '${datalinesEndPattern1}' markers`, () => {
const text = `%put hello\n${datalinesBeginPattern3}\n${datalines}\n${datalinesEndPattern1}\n%put world;`
expect(getDataSectionsDetail(text, config)).toEqual([
{
start: 1,
end: 5
}
])
})

it(`should return the detail of data section when it begins and ends with '${datalinesBeginPattern4}' and '${datalinesEndPattern2}' markers`, () => {
const text = `%put hello\n${datalinesBeginPattern4}\n${datalines}\n${datalinesEndPattern2}\n%put world;`
expect(getDataSectionsDetail(text, config)).toEqual([
{
start: 1,
end: 5
}
])
})

it(`should return the detail of data section when it begins and ends with '${datalinesBeginPattern5}' and '${datalinesEndPattern1}' markers`, () => {
const text = `%put hello\n${datalinesBeginPattern5}\n${datalines}\n${datalinesEndPattern1}\n%put world;`
expect(getDataSectionsDetail(text, config)).toEqual([
{
start: 1,
end: 5
}
])
})

it(`should return the detail of data section when it begins and ends with '${datalinesBeginPattern6}' and '${datalinesEndPattern2}' markers`, () => {
const text = `%put hello\n${datalinesBeginPattern6}\n${datalines}\n${datalinesEndPattern2}\n%put world;`
expect(getDataSectionsDetail(text, config)).toEqual([
{
start: 1,
end: 5
}
])
})
})

describe('checkIsDataLine', () => {
const config = new LintConfig(DefaultLintConfiguration)
it(`should return true if a line index is in a range of any data section`, () => {
const text = `%put hello\n${datalinesBeginPattern1}\n${datalines}\n${datalinesEndPattern1}\n%put world;`
expect(
checkIsDataLine(
[
{
start: 1,
end: 5
}
],
4
)
).toBe(true)
})

it(`should return false if a line index is not in a range of any of data sections`, () => {
const text = `%put hello\n${datalinesBeginPattern1}\n${datalines}\n${datalinesEndPattern1}\n%put world;`
expect(
checkIsDataLine(
[
{
start: 1,
end: 5
}
],
8
)
).toBe(false)
})
})
Loading