Skip to content
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
14 changes: 11 additions & 3 deletions bin/tempo-lints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
generateConfigContent,
getRuleDirs,
getRuleDirsRelative,
getValidRuleIds,
isValidLanguage,
LANG,
type Language,
Expand Down Expand Up @@ -47,7 +48,7 @@ function runScan(language: string, scanPath: string, options: ScanOptions): void

const excludeRules = options.exclude?.split(',').map((r) => r.trim()) ?? []

runAstGrep(configPath, scanPath, {
runAstGrep(language, configPath, scanPath, {
json: options.json ?? false,
fix: options.fix ?? false,
githubAction: options.githubAction ?? false,
Expand All @@ -64,7 +65,12 @@ interface AstGrepOptions {
cleanup: () => void
}

function runAstGrep(configPath: string, scanPath: string, options: AstGrepOptions): void {
function runAstGrep(
language: Language,
configPath: string,
scanPath: string,
options: AstGrepOptions,
): void {
const { cleanup } = options

const handleSignal = (signal: 'SIGINT' | 'SIGTERM') => {
Expand Down Expand Up @@ -108,7 +114,9 @@ function runAstGrep(configPath: string, scanPath: string, options: AstGrepOption
cleanup()

if (useJson && output) {
const { issues, error } = parseLintIssues(output)
// Get valid rule IDs to filter out non-tempo lint entries
const validRuleIds = getValidRuleIds(language)
const { issues, error } = parseLintIssues(output, validRuleIds)

if (error) {
warn(error)
Expand Down
111 changes: 111 additions & 0 deletions scripts/action-integration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* Integration tests for the GitHub Action
* Tests the full flow: lint → JSON output → PR comment generation
*/

import { execSync } from 'node:child_process'
import path from 'node:path'
import { describe, expect, it } from 'vitest'
import { countBySeverity, type LintIssue, PACKAGE_ROOT, parseLintIssues } from './shared.ts'

const FIXTURES_DIR = path.join(PACKAGE_ROOT, 'test-fixtures/rust')
const CLI_PATH = path.join(PACKAGE_ROOT, 'bin/tempo-lints.ts')

function runLintsOnFixtures(): LintIssue[] {
try {
const output = execSync(`pnpm exec tsx "${CLI_PATH}" rust "${FIXTURES_DIR}" --json`, {
encoding: 'utf8',
cwd: PACKAGE_ROOT,
stdio: ['pipe', 'pipe', 'pipe'],
})

const { issues, error } = parseLintIssues(output)
if (error) {
throw new Error(`Failed to parse lint output: ${error}`)
}
return issues
} catch (err) {
// execSync throws on non-zero exit, but we still want the output
const error = err as { stdout?: string }
if (error.stdout) {
const { issues, error: parseError } = parseLintIssues(error.stdout)
if (!parseError) {
return issues
}
}
throw err
}
}

describe('Action Integration Tests', () => {
it('should find lint violations in test fixtures', () => {
const issues = runLintsOnFixtures()
expect(issues.length).toBeGreaterThan(0)
})

it('should only return tempo lint rule IDs (not other tools)', () => {
const issues = runLintsOnFixtures()

// All rule IDs should be from tempo lints, not other tools
for (const issue of issues) {
expect(issue.ruleId).not.toMatch(/^clippy::/)
expect(issue.ruleId).not.toBe('unknown')
}
})

it('should find dbg! macro violations', () => {
const issues = runLintsOnFixtures()
const dbgIssues = issues.filter((issue) => issue.ruleId === 'no-dbg-macro')
expect(dbgIssues.length).toBeGreaterThan(0)
})

it('should find unwrap violations', () => {
const issues = runLintsOnFixtures()
const unwrapIssues = issues.filter((issue) => issue.ruleId === 'no-unwrap-in-lib')
expect(unwrapIssues.length).toBeGreaterThan(0)
})

it('should return issues with all required fields', () => {
const issues = runLintsOnFixtures()

for (const issue of issues) {
expect(issue.ruleId).toBeTruthy()
expect(issue.file).toBeTruthy()
expect(issue.line).toBeGreaterThan(0)
expect(issue.severity).toBeTruthy()
expect(issue.message).toBeTruthy()
}
})

it('should correctly count issues by severity', () => {
const issues = runLintsOnFixtures()
const counts = countBySeverity(issues)

expect(counts.error).toBeGreaterThanOrEqual(0)
expect(counts.warning).toBeGreaterThanOrEqual(0)
expect(counts.hint).toBeGreaterThanOrEqual(0)
expect(counts.error + counts.warning + counts.hint).toBe(issues.length)
})

it('should generate valid JSON output', () => {
let output: string
try {
output = execSync(`pnpm exec tsx "${CLI_PATH}" rust "${FIXTURES_DIR}" --json`, {
encoding: 'utf8',
cwd: PACKAGE_ROOT,
stdio: ['pipe', 'pipe', 'pipe'],
})
} catch (err) {
// Command exits with non-zero when errors are found, but still outputs JSON
const error = err as { stdout?: string }
output = error.stdout || ''
}

// Should be valid JSON
expect(() => JSON.parse(output)).not.toThrow()

// Should be an array
const parsed = JSON.parse(output)
expect(Array.isArray(parsed)).toBe(true)
})
})
106 changes: 106 additions & 0 deletions scripts/shared.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
generateConfigContent,
getRuleDirs,
getRuleDirsRelative,
getValidRuleIds,
isValidLanguage,
LANG,
type LintIssue,
Expand Down Expand Up @@ -511,3 +512,108 @@ describe('LANG constants', () => {
expect(LANG.ALL).toBe('all')
})
})

describe('getValidRuleIds', () => {
it('should return rust rule IDs', () => {
const ids = Array.from(getValidRuleIds(LANG.RUST))

// Should include known rust rules
expect(ids).toContain('no-dbg-macro')
expect(ids).toContain('no-unwrap-in-lib')
expect(ids).toContain('no-mem-transmute')
expect(ids).toContain('tracing-no-format')
expect(ids).toContain('unsafe-needs-safety-comment')
})

it('should return typescript rule IDs', () => {
const ids = Array.from(getValidRuleIds(LANG.TYPESCRIPT))

// Should include known typescript rules (if any exist)
expect(ids.length).toBeGreaterThanOrEqual(0)
})

it('should return all rule IDs for "all" language', () => {
const ids = Array.from(getValidRuleIds(LANG.ALL))

// Should include rules from both languages
expect(ids).toContain('no-dbg-macro')
expect(ids.length).toBeGreaterThan(0)
})
})

describe('parseLintIssues with rule ID filtering', () => {
it('should filter out entries with invalid ruleId', () => {
const mixedOutput = JSON.stringify([
{
ruleId: 'no-dbg-macro',
file: 'test.rs',
range: { start: { line: 1, column: 1 }, end: { line: 1, column: 10 } },
message: 'Remove dbg! macro',
severity: 'error',
},
{
ruleId: 'clippy::unwrap_used',
file: 'test.rs',
range: { start: { line: 2, column: 1 }, end: { line: 2, column: 10 } },
message: 'Clippy warning',
severity: 'warning',
},
{
ruleId: 'unknown-rule',
file: 'test.rs',
range: { start: { line: 3, column: 1 }, end: { line: 3, column: 10 } },
message: 'Unknown',
severity: 'warning',
},
])

const validIds = new Set(['no-dbg-macro', 'no-unwrap-in-lib'])
const result = parseLintIssues(mixedOutput, validIds)

expect(result.error).toBeNull()
expect(result.issues).toHaveLength(1)
expect(result.issues[0]?.ruleId).toBe('no-dbg-macro')
})

it('should pass through all entries when no validRuleIds provided', () => {
const output = JSON.stringify([
{
ruleId: 'no-dbg-macro',
file: 'test.rs',
range: { start: { line: 1, column: 1 }, end: { line: 1, column: 10 } },
message: 'Remove dbg! macro',
severity: 'error',
},
{
ruleId: 'unknown-rule',
file: 'test.rs',
range: { start: { line: 2, column: 1 }, end: { line: 2, column: 10 } },
message: 'Unknown',
severity: 'warning',
},
])

const result = parseLintIssues(output)

expect(result.error).toBeNull()
expect(result.issues).toHaveLength(2)
})

it('should filter all entries if none match validRuleIds', () => {
const output = JSON.stringify([
{
ruleId: 'clippy::unwrap_used',
file: 'test.rs',
range: { start: { line: 1, column: 1 }, end: { line: 1, column: 10 } },
message: 'Clippy warning',
severity: 'warning',
},
])

const validIds = new Set(['no-dbg-macro'])
const result = parseLintIssues(output, validIds)

expect(result.error).toBeNull()
expect(result.issues).toHaveLength(0)
})
})
61 changes: 57 additions & 4 deletions scripts/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ function isAstGrepIssueArray(value: unknown): value is AstGrepIssue[] {
for (let i = 0; i < samplesToCheck; i++) {
const item = value[i]
if (typeof item !== 'object' || item === null) return false
// ast-grep always includes file and range
if (!('file' in item || 'ruleId' in item)) {
// Require ruleId field to ensure we only accept lint rule violations
// This filters out parse errors and other non-lint entries
if (!('ruleId' in item)) {
return false
}
}
Expand All @@ -96,7 +97,10 @@ function normalizeIssue(issue: AstGrepIssue): LintIssue {
}
}

export function parseLintIssues(input: string): { issues: LintIssue[]; error: string | null } {
export function parseLintIssues(
input: string,
validRuleIds?: Set<string>,
): { issues: LintIssue[]; error: string | null } {
const { data, error } = safeParseJSON<unknown>(input)

if (error) {
Expand All @@ -108,7 +112,24 @@ export function parseLintIssues(input: string): { issues: LintIssue[]; error: st
}

// Normalize ast-grep issues to our format
const issues = data.map(normalizeIssue)
let issues = data.map(normalizeIssue)

// Filter by valid rule IDs if provided (to exclude non-tempo lint entries)
if (validRuleIds) {
const filteredOut: LintIssue[] = []
issues = issues.filter((issue) => {
if (validRuleIds.has(issue.ruleId)) {
return true
}
filteredOut.push(issue)
return false
})

// Warn about filtered issues
for (const issue of filteredOut) {
warn(`Filtered out non-tempo lint issue: ${issue.ruleId} in ${issue.file}:${issue.line}`)
}
}

return { issues, error: null }
}
Expand Down Expand Up @@ -149,6 +170,38 @@ export function getRuleDirsRelative(language: Language): string[] {
return dirs
}

/**
* Get all valid rule IDs for a given language by scanning rule directories.
* This is used to filter out non-lint entries from the output.
*/
export function getValidRuleIds(
language: Language,
packageRoot: string = PACKAGE_ROOT,
): Set<string> {
const ruleDirs = getRuleDirs(language, packageRoot)
const ids = new Set<string>()

for (const dir of ruleDirs) {
if (!fs.existsSync(dir)) {
continue
}

const files = fs.readdirSync(dir)
for (const file of files) {
if (file.endsWith('.yml')) {
const filePath = path.join(dir, file)
const content = fs.readFileSync(filePath, 'utf8')
const match = content.match(/^id:\s*(.+)$/m)
if (match?.[1]) {
ids.add(match[1].trim())
}
}
}
}

return ids
}

export function generateConfigContent(ruleDirs: string[]): string {
return `ruleDirs:\n${ruleDirs.map((d) => ` - ${d}`).join('\n')}\n`
}
Expand Down
14 changes: 14 additions & 0 deletions test-fixtures/rust/mixed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// This file contains multiple types of violations

pub fn process_data(input: Option<String>) -> String {
let data = input.unwrap(); // no-unwrap-in-lib violation

dbg!(&data); // no-dbg-macro violation

data.to_uppercase()
}

pub fn debug_value(x: i32) {
dbg!(x); // Another dbg! violation
println!("Value: {}", x);
}
19 changes: 19 additions & 0 deletions test-fixtures/rust/valid.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// This file should have no lint violations

fn add(a: i32, b: i32) -> i32 {
a + b
}

fn multiply(a: i32, b: i32) -> i32 {
a * b
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_add() {
assert_eq!(add(2, 3), 5);
}
}
Loading