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

fix: Adjust the first line of a patch when it has 0 indent #2097

Closed
wants to merge 2 commits into from
Closed
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
104 changes: 86 additions & 18 deletions packages/cli/src/rpc/file/applyFileUpdate.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { verbose } from '../../utils';
import { readFile, writeFile } from 'fs/promises';
import { warn } from 'console';
import assert from 'assert';
import makeDebug from 'debug';

const debug = makeDebug('appmap:cli:file-update');

function findLineMatch(
haystack: readonly string[],
Expand Down Expand Up @@ -47,35 +48,102 @@ function makeWhitespaceAdjuster(to: string, from: string) {
return adjuster;
}

type MatchResult = {
index: number;
length: number;
whitespaceAdjuster: (s: string) => string;
whitespaceAdjusterDescription: string;
};

function matchFileUpdate(fileLines: string[], originalLines: string[]): MatchResult | undefined {
const match = findLineMatch(fileLines, originalLines);
if (!match) return undefined;

const [index, length] = match;
const nonEmptyIndex = originalLines.findIndex((s) => s.trim());
const adjustWhitespace = makeWhitespaceAdjuster(
fileLines[index + nonEmptyIndex],
originalLines[nonEmptyIndex]
);

return {
index,
length,
whitespaceAdjuster: adjustWhitespace,
whitespaceAdjusterDescription: adjustWhitespace.desc,
};
}

function searchForFileUpdate(
whitespaceAdjustments: string[],
fileLines: string[],
originalLines: string[]
): [MatchResult, string] | undefined {
for (const whitespaceAdjustment of whitespaceAdjustments) {
const adjustedOriginalLines = [...originalLines];
adjustedOriginalLines[0] = whitespaceAdjustment + adjustedOriginalLines[0];
const match = matchFileUpdate(fileLines, adjustedOriginalLines);
if (match) return [match, whitespaceAdjustment];
}

return undefined;
}

export default async function applyFileUpdate(
file: string,
original: string,
modified: string
): Promise<string[] | undefined> {
// Read the original file
const fileContents = await readFile(file, 'utf-8');
const fileLines = fileContents.split('\n');

const originalLines = original.split('\n');

const match = findLineMatch(fileLines, originalLines);
if (!match) return [`[file-update] Failed to find match for ${file}.\n`];
if (fileLines.length === 0) {
debug(`File is empty. Skipping.`);
return undefined;
}
if (originalLines.length === 0) {
debug(`Original text is empty. Skipping.`);
return undefined;
}

const [index, length] = match;
const firstLineLeadingWhitespace = originalLines[0].match(/^\s*/)?.[0];
let whitespaceAdjustments: Set<string>;

const nonEmptyIndex = originalLines.findIndex((s) => s.trim());
const adjustWhitespace = makeWhitespaceAdjuster(
fileLines[index + nonEmptyIndex],
originalLines[nonEmptyIndex]
if (!firstLineLeadingWhitespace) {
debug(
`No leading whitespace found in the first line of the original text. Will attempt a fuzzy match.`
);
const fileLinesMatchingFirstOriginalLine = new Set(
fileLines.filter((line) => line.includes(originalLines[0]))
);
whitespaceAdjustments = new Set(
Array.from(fileLinesMatchingFirstOriginalLine).map((line) => line.match(/^\s*/)?.[0] ?? '')
);
} else {
whitespaceAdjustments = new Set(['']);
}

const searchResult = searchForFileUpdate(
Array.from(whitespaceAdjustments),
fileLines,
originalLines
);
if (!searchResult) {
debug(`No match found for the original text.`);
return undefined;
}

if (verbose())
warn(
`[file-update] Found match at line ${index + 1}, whitespace adjustment: ${
adjustWhitespace.desc
}\n`
);
fileLines.splice(index, length, ...modified.split('\n').map(adjustWhitespace));
const [match, leadingWhitespace] = searchResult;
const adjustedModified = [...modified.split('\n')];
adjustedModified[0] = leadingWhitespace + adjustedModified[0];

debug(
`[file-update] Found match at line ${match.index + 1}, whitespace adjustment: ${
match.whitespaceAdjusterDescription
}\n`
);
fileLines.splice(match.index, match.length, ...adjustedModified.map(match.whitespaceAdjuster));

await writeFile(file, fileLines.join('\n'), 'utf8');
}
16 changes: 6 additions & 10 deletions packages/cli/tests/unit/rpc/file/applyFileUpdate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import tmp from 'tmp';

import applyFileUpdate from '../../../../src/rpc/file/applyFileUpdate';
import { load } from 'js-yaml';
import assert from 'node:assert';

type Change = {
file: string;
Expand Down Expand Up @@ -37,17 +36,9 @@ describe(applyFileUpdate, () => {
await applyFileUpdate(file, original, modified);

const updatedStr = await readFile('original.txt', 'utf8');
const updated = updatedStr
.split('\n')
.map((line) => line.trim())
.join('\n');
const expectedStr = await readFile('expected.txt', 'utf8');
const expected = expectedStr
.split('\n')
.map((line) => line.trim())
.join('\n');

expect(updated).toEqual(expected);
expect(updatedStr).toEqual(expectedStr);
};

it('correctly applies an update even with broken whitespace', example('whitespace-mismatch'));
Expand All @@ -57,4 +48,9 @@ describe(applyFileUpdate, () => {
'correctly applies an update even when there are repeated similar but mismatching lines',
example('mismatched-similar')
);
it('compensates for missing indentation in the first line', example('missing-firstline-indent'));
it(
'compensates for missing indentation in the first line (even when there is a mismatch)',
example('missing-firstline-indent-must-match')
);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
file: original.txt
original: |-
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields
modified: |-
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
if hasattr(field, '_bind_to_schema'):
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# hello
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
if hasattr(field, '_bind_to_schema'):
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# hello
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
file: original.txt
original: |-
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields
modified: |-
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
if hasattr(field, '_bind_to_schema'):
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# hello
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
if hasattr(field, '_bind_to_schema'):
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# hello
def _bind_to_schema(self, field_name, schema):
super()._bind_to_schema(field_name, schema)
new_tuple_fields = []
for field in self.tuple_fields:
field = copy.deepcopy(field)
field._bind_to_schema(field_name, self)
new_tuple_fields.append(field)

self.tuple_fields = new_tuple_fields

Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,24 @@
# A buffer for all table expressions in join conditions
from_expression_elements = []
column_reference_segments = []

from_clause_segment = segment.get_child("from_clause")

if not from_clause_segment:
return None

# Check if the FROM clause contains any JOINs
join_clauses = from_clause_segment.recursive_crawl('join_clause')
if not join_clauses:
return None

from_expression = from_clause_segment.get_child("from_expression")
from_expression_element = None
if from_expression:
from_expression_element = from_expression.get_child(
"from_expression_element"
)

if not from_expression_element:
return None
from_expression_element = from_expression_element.get_child(
Expand Down
Loading