Repository navigation
Disallow unsafe returns. #19767
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
Disallow unsafe returns. #19767
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -83,6 +83,7 @@ export class ExtensionRegistryClient { | |||||||
| }); | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||||||||
| const results = await fzf.find(query); | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return results.map((r: { item: RegistryExtension }) => r.item); | ||||||||
|
Comment on lines
+86
to
87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of disabling the
Suggested change
|
||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -179,6 +179,7 @@ export class ExtensionEnablementManager { | |||||||
| readConfig(): AllExtensionsEnablementConfig { | ||||||||
| try { | ||||||||
| const content = fs.readFileSync(this.configFilePath, 'utf-8'); | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return JSON.parse(content); | ||||||||
|
Comment on lines
+182
to
183
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } catch (error) { | ||||||||
| if ( | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,6 +156,7 @@ export async function promptForSetting( | |
| name: 'value', | ||
| message: `${setting.name}\n${setting.description}`, | ||
| }); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return response.value; | ||
|
Comment on lines
+159
to
160
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -170,6 +170,7 @@ async function searchResourceCandidates( | |
| const results = await fzf.find(normalizedPattern, { | ||
| limit: MAX_SUGGESTIONS_TO_SHOW * 3, | ||
| }); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return results.map( | ||
| (result: { item: ResourceSuggestionCandidate }) => result.item.suggestion, | ||
| ); | ||
|
Comment on lines
+173
to
176
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of disabling the return (results as Array<{ item: ResourceSuggestionCandidate }>).map(
(result) => result.item.suggestion,
); |
||
|
|
@@ -193,6 +194,7 @@ async function searchAgentCandidates( | |
| const results = await fzf.find(normalizedPattern, { | ||
| limit: MAX_SUGGESTIONS_TO_SHOW, | ||
| }); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return results.map((r: { item: Suggestion }) => r.item); | ||
|
Comment on lines
+197
to
198
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of disabling the return (results as Array<{ item: Suggestion }>).map((r) => r.item); |
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -193,8 +193,13 @@ export const TableRenderer: React.FC<TableRendererProps> = ({ | |
| const wrappedRows = styledRows.map((row) => wrapAndProcessRow(row)); | ||
|
|
||
| // Use the TIGHTEST widths that fit the wrapped content + padding | ||
| const adjustedWidths = actualColumnWidths.map((w) => w + COLUMN_PADDING); | ||
| const adjustedWidths = actualColumnWidths.map( | ||
| (w) => | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| w + COLUMN_PADDING, | ||
| ); | ||
|
Comment on lines
+196
to
+200
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of disabling the |
||
|
|
||
|
|
||
| return { wrappedHeaders, wrappedRows, adjustedWidths }; | ||
| }, [styledHeaders, styledRows, terminalWidth]); | ||
| // Helper function to render a cell with proper width | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -83,6 +83,7 @@ export const getLatestGitHubRelease = async ( | |||||||
| if (!releaseTag) { | ||||||||
| throw new Error(`Response did not include tag_name field`); | ||||||||
| } | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return releaseTag; | ||||||||
|
Comment on lines
+86
to
87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } catch (_error) { | ||||||||
| debugLogger.debug( | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ export function tryParseJSON(input: string): object | null { | |
|
|
||
| if (!Array.isArray(parsed) && Object.keys(parsed).length === 0) return null; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return parsed; | ||
|
Comment on lines
+43
to
44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } catch (_err) { | ||
| return null; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -374,6 +374,7 @@ export function setPendingSettingValue( | |
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| const newSettings = JSON.parse(JSON.stringify(pendingSettings)); | ||
| setNestedValue(newSettings, path, value); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return newSettings; | ||
|
Comment on lines
+377
to
378
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,7 @@ export class AcpFileSystemService implements FileSystemService { | |||||||
| sessionId: this.sessionId, | ||||||||
| }); | ||||||||
|
|
||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return response.content; | ||||||||
|
Comment on lines
+32
to
33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -636,6 +636,7 @@ async function fetchCachedCredentials(): Promise< | |||||||
| for (const keyFile of pathsToTry) { | ||||||||
| try { | ||||||||
| const keyFileString = await fs.readFile(keyFile, 'utf-8'); | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return JSON.parse(keyFileString); | ||||||||
|
Comment on lines
+639
to
640
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } catch (error) { | ||||||||
| // Log specific error for debugging, but continue trying other paths | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,6 +59,7 @@ export class ProjectRegistry { | |||||||
|
|
||||||||
| try { | ||||||||
| const content = await fs.promises.readFile(this.registryPath, 'utf8'); | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return JSON.parse(content); | ||||||||
|
Comment on lines
+62
to
63
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } catch (e) { | ||||||||
| debugLogger.debug('Failed to load registry: ', e); | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,21 +13,19 @@ import type { | |
| GenerateContentConfig, | ||
| } from '@google/genai'; | ||
| import type { Config } from '../config/config.js'; | ||
| import type { ContentGenerator } from './contentGenerator.js'; | ||
| import type { AuthType } from './contentGenerator.js'; | ||
| import type { ContentGenerator , AuthType } from './contentGenerator.js'; | ||
| import { handleFallback } from '../fallback/handler.js'; | ||
| import { getResponseText } from '../utils/partUtils.js'; | ||
| import { reportError } from '../utils/errorReporting.js'; | ||
| import { getErrorMessage } from '../utils/errors.js'; | ||
| import { logMalformedJsonResponse } from '../telemetry/loggers.js'; | ||
| import { MalformedJsonResponseEvent } from '../telemetry/types.js'; | ||
| import { MalformedJsonResponseEvent , LlmRole } from '../telemetry/types.js'; | ||
| import { retryWithBackoff } from '../utils/retry.js'; | ||
| import type { ModelConfigKey } from '../services/modelConfigService.js'; | ||
| import { | ||
| applyModelSelection, | ||
| createAvailabilityContextProvider, | ||
| } from '../availability/policyHelpers.js'; | ||
| import { LlmRole } from '../telemetry/types.js'; | ||
|
|
||
| const DEFAULT_MAX_ATTEMPTS = 5; | ||
|
|
||
|
|
@@ -164,6 +162,7 @@ export class BaseLlmClient { | |
| ); | ||
|
|
||
| // If we are here, the content is valid (not empty and parsable). | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||
| return JSON.parse( | ||
| this.cleanJsonResponse(getResponseText(result)!.trim(), model), | ||
| ); | ||
|
Comment on lines
+165
to
168
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -348,6 +348,7 @@ export class IdeClient { | |||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||||||||
| const parsedJson = JSON.parse(textPart.text); | ||||||||
| if (parsedJson && typeof parsedJson.content === 'string') { | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return parsedJson.content; | ||||||||
|
Comment on lines
+351
to
352
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } | ||||||||
| if (parsedJson && parsedJson.content === null) { | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -123,6 +123,7 @@ export async function getConnectionConfigFromFile( | |||||||
| `gemini-ide-server-${pid}.json`, | ||||||||
| ); | ||||||||
| const portFileContents = await fs.promises.readFile(portFile, 'utf8'); | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return JSON.parse(portFileContents); | ||||||||
|
Comment on lines
+126
to
127
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } catch (_) { | ||||||||
| // For newer extension versions, the file name matches the pattern | ||||||||
|
|
@@ -167,6 +168,7 @@ export async function getConnectionConfigFromFile( | |||||||
| } | ||||||||
| const parsedContents = fileContents.map((content) => { | ||||||||
| try { | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return JSON.parse(content); | ||||||||
|
Comment on lines
+171
to
172
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } catch (e) { | ||||||||
| logger.debug('Failed to parse JSON from config file: ', e); | ||||||||
|
|
@@ -196,6 +198,7 @@ export async function getConnectionConfigFromFile( | |||||||
| if (fileIndex !== -1) { | ||||||||
| logger.debug(`Selected IDE connection file: ${matchingFiles[fileIndex]}`); | ||||||||
| } | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return selected; | ||||||||
| } | ||||||||
|
|
||||||||
|
|
@@ -213,6 +216,7 @@ export async function getConnectionConfigFromFile( | |||||||
| `Selected IDE connection file (matched port from env): ${matchingFiles[fileIndex]}`, | ||||||||
| ); | ||||||||
| } | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return selected; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
@@ -225,6 +229,7 @@ export async function getConnectionConfigFromFile( | |||||||
| `Selected first valid IDE connection file: ${matchingFiles[fileIndex]}`, | ||||||||
| ); | ||||||||
| } | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return selected; | ||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -419,6 +419,7 @@ export class ChatRecordingService { | |||||||
| private readConversation(): ConversationRecord { | ||||||||
| try { | ||||||||
| this.cachedLastConvData = fs.readFileSync(this.conversationFile!, 'utf8'); | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-return | ||||||||
| return JSON.parse(this.cachedLastConvData); | ||||||||
|
Comment on lines
+422
to
423
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of suppressing the
Suggested change
|
||||||||
| } catch (error) { | ||||||||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of disabling the
no-unsafe-returnrule, you can ensure type safety by explicitly converting the returned value to a boolean. Theresponse.confirmproperty from thepromptslibrary is of typeany, but this function is expected to return aboolean. Using!!is a concise way to guarantee a boolean return value.