Skip to content

Commit

Permalink
PromptString linter: Only error when the reported violation is within…
Browse files Browse the repository at this point in the history
… one of the changed range (#3810)
  • Loading branch information
philipp-spiess authored Apr 16, 2024
1 parent acde6c8 commit ac5fae2
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 9 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/lints.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ jobs:
restore-keys: ${{ runner.os }}-20-pnpm-store-
- run: pnpm install
- name: Run lints
# We might want to use parallel here to batch if we run into command lengths issues
run: pnpm ts-node lints/safe-prompts.ts `git diff --name-only origin/main | grep -E "\.(ts|tsx)$"`
run: pnpm ts-node lints/safe-prompts.ts `pnpm ts-node lints/git-diff-ts-ranges.ts`
- uses: actions/github-script@v6
if: ${{ failure() }}
with:
Expand Down
51 changes: 51 additions & 0 deletions lints/git-diff-ts-ranges.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { exec } from 'node:child_process'

const EXTENSIONS = ['ts', 'tsx']

exec('git diff --unified=0 origin/main', (error, stdout, stderr) => {
if (error) {
console.error(`exec error: ${error}`)
return
}
if (stderr) {
console.error(`stderr: ${stderr}`)
return
}

// Process the output
const lines = stdout.split('\n')
let currentFile = ''
const fileRanges: { [key: string]: string[] } = {}

for (const line of lines) {
if (line.startsWith('diff --git')) {
// Extract the filename correctly after the b/ prefix
const parts = line.split(' ')
currentFile = parts[2].substring(2) // Remove the 'b/' prefix
} else if (line.startsWith('@@')) {
// Extract the line numbers for additions
const match = line.match(/\+([0-9]+),?([0-9]*)/)
if (match) {
const start = parseInt(match[1], 10)
const count = parseInt(match[2] || '1', 10)
const end = start + count - 1
if (count > 0) {
// Ensure we only add ranges where lines were added
if (!fileRanges[currentFile]) {
fileRanges[currentFile] = []
}
fileRanges[currentFile].push(`${start}-${end}`)
}
}
}
}

// Output the results
for (const [file, ranges] of Object.entries(fileRanges)) {
if (!EXTENSIONS.includes(file.split('.').pop() || '')) {
continue
}

console.log(`${file}:${ranges.join(',')}`)
}
})
41 changes: 34 additions & 7 deletions lints/safe-prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,14 @@
import { readFileSync } from 'node:fs'
import * as ts from 'typescript'

interface Range {
start: number
end: number
}

let didEncounterAnError = false

export function delint(sourceFile: ts.SourceFile) {
export function delint(sourceFile: ts.SourceFile, ranges: Range[] | null) {
delintNode(sourceFile)

function delintNode(node: ts.Node) {
Expand Down Expand Up @@ -77,14 +82,24 @@ export function delint(sourceFile: ts.SourceFile) {
}

function report(node: ts.Node, level: 'error' | 'warning', message: string) {
if (level === 'error') {
didEncounterAnError = true
}

const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart())
const { line: endLine, character: endCharacter } = sourceFile.getLineAndCharacterOfPosition(
node.getEnd()
)

// When ranges are set, only error if the reported violation is within one
// of the changed ranges.
if (ranges !== null) {
const overlappingRange = ranges.find(range => range.start <= line && range.end >= endLine)
if (overlappingRange === undefined) {
return
}
}

if (level === 'error') {
didEncounterAnError = true
}

if (process.env.GITHUB_ACTIONS !== undefined) {
console.log(
`::${level} file=${sourceFile.fileName},line=${line + 1},col=${character + 1},endLine=${
Expand All @@ -97,7 +112,19 @@ export function delint(sourceFile: ts.SourceFile) {
}

const fileNames = process.argv.slice(2)
for (const fileName of fileNames) {
for (const row of fileNames) {
const [fileName, rawRanges] = row.split(':')

const rangeStrings = rawRanges.split(',')
let ranges: Range[] | null = null
for (const rangeString of rangeStrings) {
const [start, end] = rangeString.split('-')
if (ranges === null) {
ranges = []
}
ranges.push({ start: parseInt(start), end: parseInt(end) })
}

// Parse a file
const sourceFile = ts.createSourceFile(
fileName,
Expand All @@ -107,7 +134,7 @@ for (const fileName of fileNames) {
)

// delint it
delint(sourceFile)
delint(sourceFile, ranges)
}

if (didEncounterAnError) {
Expand Down

0 comments on commit ac5fae2

Please sign in to comment.