Disallow redundant typecasts. (#15030)

This commit is contained in:
Christian Gunderman
2025-12-12 17:43:43 -08:00
committed by GitHub
parent fcc3b2b5ec
commit 942bcfc61e
86 changed files with 235 additions and 371 deletions
+2 -2
View File
@@ -156,12 +156,12 @@ describe('EditTool', () => {
const problematicSnippet =
snippetMatch && snippetMatch[1] ? snippetMatch[1] : '';
if (((schema as any).properties as any)?.corrected_target_snippet) {
if ((schema as any).properties?.corrected_target_snippet) {
return Promise.resolve({
corrected_target_snippet: problematicSnippet,
});
}
if (((schema as any).properties as any)?.corrected_new_string) {
if ((schema as any).properties?.corrected_new_string) {
// For new_string correction, we might need more sophisticated logic,
// but for now, returning original is a safe default if not specified by a test.
const originalNewStringMatch = promptText.match(
+1 -1
View File
@@ -523,7 +523,7 @@ class GrepToolInvocation extends BaseToolInvocation<
const allMatches: GrepMatch[] = [];
for await (const filePath of filesStream) {
const fileAbsolutePath = filePath as string;
const fileAbsolutePath = filePath;
try {
const content = await fsPromises.readFile(fileAbsolutePath, 'utf8');
const lines = content.split(/\r?\n/);
+2 -2
View File
@@ -184,11 +184,11 @@ export async function modifyWithEditor<ToolParams>(
overrides !== undefined && 'proposedContent' in overrides;
const currentContent = hasCurrentOverride
? (overrides!.currentContent ?? '')
? (overrides.currentContent ?? '')
: await modifyContext.getCurrentContent(originalParams);
const proposedContent = hasProposedOverride
? (overrides!.proposedContent ?? '')
? (overrides.proposedContent ?? '')
: await modifyContext.getProposedContent(originalParams);
const { oldPath, newPath, dirPath } = createTempFilesForModify(
+23 -79
View File
@@ -16,7 +16,6 @@ import type { Config } from '../config/config.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { StandardFileSystemService } from '../services/fileSystemService.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
import type { ToolInvocation, ToolResult } from './tools.js';
import { WorkspaceContext } from '../utils/workspaceContext.js';
vi.mock('../telemetry/loggers.js', () => ({
@@ -72,10 +71,7 @@ describe('ReadFileTool', () => {
};
const result = tool.build(params);
expect(typeof result).not.toBe('string');
const invocation = result as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = result;
expect(invocation.toolLocations()[0].path).toBe(
path.join(tempRootDir, 'test.txt'),
);
@@ -146,11 +142,9 @@ describe('ReadFileTool', () => {
};
const invocation = tool.build(params);
expect(typeof invocation).not.toBe('string');
expect(
(
invocation as ToolInvocation<ReadFileToolParams, ToolResult>
).getDescription(),
).toBe(path.join('sub', 'dir', 'file.txt'));
expect(invocation.getDescription()).toBe(
path.join('sub', 'dir', 'file.txt'),
);
});
it('should return shortened path when file path is deep', () => {
@@ -170,9 +164,7 @@ describe('ReadFileTool', () => {
const params: ReadFileToolParams = { file_path: deepPath };
const invocation = tool.build(params);
expect(typeof invocation).not.toBe('string');
const desc = (
invocation as ToolInvocation<ReadFileToolParams, ToolResult>
).getDescription();
const desc = invocation.getDescription();
expect(desc).toContain('...');
expect(desc).toContain('file.txt');
});
@@ -184,22 +176,16 @@ describe('ReadFileTool', () => {
};
const invocation = tool.build(params);
expect(typeof invocation).not.toBe('string');
expect(
(
invocation as ToolInvocation<ReadFileToolParams, ToolResult>
).getDescription(),
).toBe(path.join('sub', 'dir', 'file.txt'));
expect(invocation.getDescription()).toBe(
path.join('sub', 'dir', 'file.txt'),
);
});
it('should return . if path is the root directory', () => {
const params: ReadFileToolParams = { file_path: tempRootDir };
const invocation = tool.build(params);
expect(typeof invocation).not.toBe('string');
expect(
(
invocation as ToolInvocation<ReadFileToolParams, ToolResult>
).getDescription(),
).toBe('.');
expect(invocation.getDescription()).toBe('.');
});
});
@@ -209,10 +195,7 @@ describe('ReadFileTool', () => {
const fileContent = 'This is a test file.';
await fsp.writeFile(filePath, fileContent, 'utf-8');
const params: ReadFileToolParams = { file_path: 'textfile.txt' };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
expect(await invocation.execute(abortSignal)).toEqual({
llmContent: fileContent,
@@ -223,10 +206,7 @@ describe('ReadFileTool', () => {
it('should return error if file does not exist', async () => {
const filePath = path.join(tempRootDir, 'nonexistent.txt');
const params: ReadFileToolParams = { file_path: filePath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result).toEqual({
@@ -245,10 +225,7 @@ describe('ReadFileTool', () => {
const fileContent = 'This is a test file.';
await fsp.writeFile(filePath, fileContent, 'utf-8');
const params: ReadFileToolParams = { file_path: filePath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
expect(await invocation.execute(abortSignal)).toEqual({
llmContent: fileContent,
@@ -260,10 +237,7 @@ describe('ReadFileTool', () => {
const dirPath = path.join(tempRootDir, 'directory');
await fsp.mkdir(dirPath);
const params: ReadFileToolParams = { file_path: dirPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result).toEqual({
@@ -283,10 +257,7 @@ describe('ReadFileTool', () => {
const largeContent = 'x'.repeat(21 * 1024 * 1024);
await fsp.writeFile(filePath, largeContent, 'utf-8');
const params: ReadFileToolParams = { file_path: filePath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result).toHaveProperty('error');
@@ -302,10 +273,7 @@ describe('ReadFileTool', () => {
const fileContent = `Short line\n${longLine}\nAnother short line`;
await fsp.writeFile(filePath, fileContent, 'utf-8');
const params: ReadFileToolParams = { file_path: filePath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
@@ -323,10 +291,7 @@ describe('ReadFileTool', () => {
]);
await fsp.writeFile(imagePath, pngHeader);
const params: ReadFileToolParams = { file_path: imagePath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toEqual({
@@ -344,10 +309,7 @@ describe('ReadFileTool', () => {
const pdfHeader = Buffer.from('%PDF-1.4');
await fsp.writeFile(pdfPath, pdfHeader);
const params: ReadFileToolParams = { file_path: pdfPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toEqual({
@@ -365,10 +327,7 @@ describe('ReadFileTool', () => {
const binaryData = Buffer.from([0x00, 0xff, 0x00, 0xff]);
await fsp.writeFile(binPath, binaryData);
const params: ReadFileToolParams = { file_path: binPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe(
@@ -382,10 +341,7 @@ describe('ReadFileTool', () => {
const svgContent = '<svg><circle cx="50" cy="50" r="40"/></svg>';
await fsp.writeFile(svgPath, svgContent, 'utf-8');
const params: ReadFileToolParams = { file_path: svgPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe(svgContent);
@@ -398,10 +354,7 @@ describe('ReadFileTool', () => {
const largeContent = '<svg>' + 'x'.repeat(1024 * 1024 + 1) + '</svg>';
await fsp.writeFile(svgPath, largeContent, 'utf-8');
const params: ReadFileToolParams = { file_path: svgPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe(
@@ -416,10 +369,7 @@ describe('ReadFileTool', () => {
const emptyPath = path.join(tempRootDir, 'empty.txt');
await fsp.writeFile(emptyPath, '', 'utf-8');
const params: ReadFileToolParams = { file_path: emptyPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe('');
@@ -437,10 +387,7 @@ describe('ReadFileTool', () => {
offset: 5, // Start from line 6
limit: 3,
};
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain(
@@ -465,10 +412,7 @@ describe('ReadFileTool', () => {
await fsp.writeFile(tempFilePath, tempFileContent, 'utf-8');
const params: ReadFileToolParams = { file_path: tempFilePath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
const invocation = tool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe(tempFileContent);
+2 -2
View File
@@ -147,12 +147,12 @@ describe('SmartEditTool', () => {
const problematicSnippet =
snippetMatch && snippetMatch[1] ? snippetMatch[1] : '';
if (((schema as any).properties as any)?.corrected_target_snippet) {
if ((schema as any).properties?.corrected_target_snippet) {
return Promise.resolve({
corrected_target_snippet: problematicSnippet,
});
}
if (((schema as any).properties as any)?.corrected_new_string) {
if ((schema as any).properties?.corrected_new_string) {
const originalNewStringMatch = promptText.match(
/original_new_string \(what was intended to replace original_old_string\):\n```\n([\s\S]*?)\n```/,
);
+1 -1
View File
@@ -536,7 +536,7 @@ describe('WriteFileTool', () => {
expect(confirmation.onConfirm).toBeDefined();
// Call `onConfirm` to trigger the logic that updates the content
await confirmation.onConfirm!(ToolConfirmationOutcome.ProceedOnce);
await confirmation.onConfirm(ToolConfirmationOutcome.ProceedOnce);
// Now, check if the original `params` object (captured by the invocation) was modified
expect(invocation.params.content).toBe('ide-modified-content');