Skip to content

Commit e4790c0

Browse files
authored
Bugfixes and upgraded to v0.0.5 (#7)
* Bugfix: Support octal-escaped filenames coming from git diff. * Bugfix: don't try to open directories, better handling of `--- ...` non-filename diffs. * Bugfix: Warn instead of failing if the comment extractor sees an unknown file.
1 parent e321be7 commit e4790c0

File tree

5 files changed

+176
-7
lines changed

5 files changed

+176
-7
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "ifttt-lint",
3-
"version": "0.0.4",
3+
"version": "0.0.5",
44
"description": "A high-concurrency IFTTT (IfThisThenThat) linter",
55
"homepage": "https://www.github.com/ebrevdo/ifttt-lint",
66
"main": "dist/main.js",

src/DiffParser.ts

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,18 @@ import parseDiff from 'parse-diff';
3939
* @returns A Map where each key is a file path and its value contains added and removed line numbers.
4040
*/
4141
export function parseChangedLines(diffText: string): Map<string, FileChanges> {
42-
// Remove 'diff ' headers to avoid parse-diff header misparsing
42+
// Sanitize diff input: drop git diff headers and spurious '--- ' or '+++ ' lines not indicating actual file paths
4343
const filtered = diffText
4444
.split(/\r?\n/)
45-
.filter(line => !line.startsWith('diff '))
45+
.filter(line => {
46+
// drop main diff header lines
47+
if (line.startsWith('diff ')) return false;
48+
// drop spurious file-header-like lines: raw '--- ' not followed by a valid file path (e.g., prefix/ or /dev/null)
49+
if (/^--- /.test(line) && !/^--- [^ ]+\//.test(line)) return false;
50+
// drop spurious new-file-header-like lines: raw '+++ ' not followed by a valid file path (e.g., prefix/ or /dev/null)
51+
if (/^\+\+\+ /.test(line) && !/^\+\+\+ [^ ]+\//.test(line)) return false;
52+
return true;
53+
})
4654
.join('\n');
4755
const files = parseDiff(filtered);
4856
const result = new Map<string, FileChanges>();
@@ -52,7 +60,42 @@ export function parseChangedLines(diffText: string): Map<string, FileChanges> {
5260
continue;
5361
}
5462
// Determine file path: prefer 'to' unless it's '/dev/null', else use 'from'
55-
const raw = file.to && file.to !== '/dev/null' ? file.to : file.from;
63+
let raw = file.to && file.to !== '/dev/null' ? file.to : file.from;
64+
raw = raw.trim();
65+
// Strip surrounding quotes if present
66+
if ((raw.startsWith('"') && raw.endsWith('"')) || (raw.startsWith("'") && raw.endsWith("'"))) {
67+
raw = raw.slice(1, -1);
68+
}
69+
// Decode C-style octal escapes (e.g., \360) to actual UTF-8 characters
70+
raw = ((): string => {
71+
const bytes: number[] = [];
72+
let i = 0;
73+
while (i < raw.length) {
74+
if (raw[i] === '\\') {
75+
let j = i + 1;
76+
let oct = '';
77+
// collect up to 3 octal digits
78+
while (j < raw.length && oct.length < 3 && /[0-7]/.test(raw[j])) {
79+
oct += raw[j];
80+
j++;
81+
}
82+
if (oct.length > 0) {
83+
bytes.push(parseInt(oct, 8));
84+
i = j;
85+
continue;
86+
}
87+
// not an octal escape, treat literal '\'
88+
bytes.push(raw.charCodeAt(i));
89+
i++;
90+
} else {
91+
// normal character: encode as UTF-8 bytes
92+
const buf = Buffer.from(raw[i], 'utf-8');
93+
for (const b of buf) bytes.push(b);
94+
i++;
95+
}
96+
}
97+
return Buffer.from(bytes).toString('utf-8');
98+
})();
5699
// Strip single-character prefix (e.g., 'a/', 'b/')
57100
const filePath = raw.length > 1 && raw[1] === '/' ? raw.slice(2) : raw;
58101
const added = new Set<number>();

src/DirectiveParser.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,29 @@ const endLabelRegex = /LINT\.EndLabel/;
2727
export async function parseFileDirectives(
2828
filePath: string
2929
): Promise<LintDirective[]> {
30-
const content = await fs.readFile(filePath, 'utf-8');
30+
let content: string;
31+
try {
32+
content = await fs.readFile(filePath, 'utf-8');
33+
} catch (err: unknown) {
34+
// If path is a directory, skip without error
35+
if (
36+
typeof err === 'object' &&
37+
err !== null &&
38+
'code' in err &&
39+
(err as { code?: string }).code === 'EISDIR'
40+
) {
41+
return [];
42+
}
43+
// Propagate other errors (e.g., permission issues)
44+
throw err;
45+
}
3146
// Use multi-language comment extractor to find all comments in source
3247
// Pass filename so extractor can choose comment syntax by extension
3348
let commentsMap: Record<string, { content?: string }>;
49+
// Try extracting comments by file extension; if unsupported, fall back or warn
3450
try {
3551
commentsMap = extractComments(content, { filename: filePath });
3652
} catch {
37-
// Fallback for unsupported extensions (e.g., .bzl): remap to Python or JS
3853
const ext = path.extname(filePath).toLowerCase();
3954
let fallback: string;
4055
if (ext === '.bzl') {
@@ -44,7 +59,12 @@ export async function parseFileDirectives(
4459
// Default to JavaScript
4560
fallback = filePath.replace(path.extname(filePath), '.js');
4661
}
47-
commentsMap = extractComments(content, { filename: fallback });
62+
try {
63+
commentsMap = extractComments(content, { filename: fallback });
64+
} catch {
65+
// Could not extract comments (e.g., JSON or unsupported extensions): ignore silently
66+
return [];
67+
}
4868
}
4969
// commentsMap maps starting line numbers (as strings) to comment objects
5070
const directives: LintDirective[] = [];

tests/DiffParser.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import { parseChangedLines } from '../src/DiffParser';
2+
3+
describe('parseChangedLines spurious header filtering', () => {
4+
it('ignores comment lines starting with --- without valid path', () => {
5+
const diff = [
6+
'diff --git a/file.sql b/file.sql',
7+
'index abc..def 100644',
8+
'--- a/file.sql',
9+
'+++ b/file.sql',
10+
'@@ -1,6 +1,6 @@',
11+
' SELECT *',
12+
'+SELECT hello',
13+
' --- This is a comment, not a file header',
14+
'-OLD LINE',
15+
'+NEW LINE'
16+
].join('\n');
17+
const result = parseChangedLines(diff);
18+
// Only one file should be parsed
19+
expect(result.size).toBe(1);
20+
expect(result.has('file.sql')).toBe(true);
21+
const changes = result.get('file.sql')!;
22+
// Added lines at newLine positions 2 and 4
23+
expect(changes.addedLines.has(2)).toBe(true);
24+
expect(changes.addedLines.has(4)).toBe(true);
25+
// Removed line at oldLine position 3
26+
expect(changes.removedLines.has(3)).toBe(true);
27+
});
28+
29+
it('retains real file header lines starting with --- a/ or +++ b/', () => {
30+
const diff = [
31+
'diff --git a/foo.js b/foo.js',
32+
'index 123..456 100644',
33+
'--- a/foo.js',
34+
'+++ b/foo.js',
35+
'@@ -0,0 +1,1 @@',
36+
'+console.log("hi");'
37+
].join('\n');
38+
const result = parseChangedLines(diff);
39+
expect(result.size).toBe(1);
40+
expect(Array.from(result.keys())).toEqual(['foo.js']);
41+
const changes = result.get('foo.js')!;
42+
expect(changes.addedLines.has(1)).toBe(true);
43+
});
44+
it('accepts file headers with arbitrary prefixes (e.g., c/ and w/)', () => {
45+
const diff = [
46+
'diff --git c/path/file.txt w/path/file.txt',
47+
'index 123..456 100644',
48+
'--- c/path/file.txt',
49+
'+++ w/path/file.txt',
50+
'@@ -1 +1 @@',
51+
'-old line',
52+
'+new line'
53+
].join('\n');
54+
const result = parseChangedLines(diff);
55+
expect(result.size).toBe(1);
56+
expect(result.has('path/file.txt')).toBe(true);
57+
const changes = result.get('path/file.txt')!;
58+
expect(changes.removedLines.has(1)).toBe(true);
59+
expect(changes.addedLines.has(1)).toBe(true);
60+
});
61+
62+
it('strips surrounding quotes and prefixes for quoted file paths', () => {
63+
const diff = [
64+
'diff --git "a/my file.txt" "b/my file.txt"',
65+
'index 111..222 100644',
66+
'--- "a/my file.txt"',
67+
'+++ "b/my file.txt"',
68+
'@@ -1 +1 @@',
69+
'-old',
70+
'+new'
71+
].join('\n');
72+
const result = parseChangedLines(diff);
73+
expect(result.size).toBe(1);
74+
expect(result.has('my file.txt')).toBe(true);
75+
const changes = result.get('my file.txt')!;
76+
expect(changes.removedLines.has(1)).toBe(true);
77+
expect(changes.addedLines.has(1)).toBe(true);
78+
});
79+
80+
it('decodes octal-escaped paths with non-ASCII chars', () => {
81+
const esc = '3_\\360\\237\\224\\216_test.py';
82+
const diff = [
83+
`diff --git a/${esc} b/${esc}`,
84+
'index 1..2 100644',
85+
`--- a/${esc}`,
86+
`+++ b/${esc}`,
87+
'@@ -1 +1 @@',
88+
'-old',
89+
'+new'
90+
].join('\n');
91+
const result = parseChangedLines(diff);
92+
const expectedName = '3_🔎_test.py';
93+
expect(result.size).toBe(1);
94+
expect(result.has(expectedName)).toBe(true);
95+
const changes = result.get(expectedName)!;
96+
expect(changes.removedLines.has(1)).toBe(true);
97+
expect(changes.addedLines.has(1)).toBe(true);
98+
});
99+
});

tests/DirectiveParserList.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,11 @@ describe('DirectiveParser ThenChange array literal', () => {
4848
{ kind: 'ThenChange', line: 1, target: 'file2.ts#lbl' }
4949
]);
5050
});
51+
describe('parseFileDirectives on directories', () => {
52+
it('returns empty directive list for directory paths', async () => {
53+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'lint-dir-'));
54+
const directives = await parseFileDirectives(tmpDir);
55+
expect(directives).toEqual([]);
56+
});
57+
});
5158
});

0 commit comments

Comments
 (0)