Skip to content
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
38 changes: 38 additions & 0 deletions .roo/rules/eslint-no-empty.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Handling ESLint `no-empty` Rule

Empty block statements (`{}`) are disallowed. All blocks MUST include a logging statement (e.g., `console.error`, `console.warn`, `console.log`, `console.debug`) to make their purpose explicit.

This applies to `if`, `else`, `while`, `for`, `switch` cases, `try...catch` blocks, and function bodies.

### Examples:

```javascript
// Correct: Logging in blocks
if (condition) {
console.warn("Condition met, no specific action.")
}

try {
criticalOperation()
} catch (error) {
// For unexpected errors:
console.error("Unexpected error in criticalOperation:", error)
}

function foo() {
console.log("foo called, no operation.")
}
```

### Special Considerations:

- **Intentional Error Suppression**: Use `console.debug` in `catch` blocks if an error is intentionally suppressed.
```javascript
try {
operationThatMightBenignlyFail()
} catch (error) {
console.debug("Benign failure in operation, suppressed:", error)
}
```
- **Constructors**: Empty constructors should log, preferably with `console.debug`.
- **Comments**: Comments can supplement logs but do not replace the logging requirement.
85 changes: 85 additions & 0 deletions .roo/rules/eslint-no-implicit-coercion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Handling ESLint `no-implicit-coercion` Errors

When ESLint's `no-implicit-coercion` rule flags an error, a value is being implicitly converted to a boolean, number, or string. While ESLint might suggest `Boolean(value)`, `Number(value)`, or `String(value)`, this project requires a different approach for boolean coercions.

## Guideline: Explicit, Context-Aware Comparisons

For boolean coercions (e.g., in `if` statements or ternaries), MUST NOT use `Boolean(value)`. Instead, extend conditional expressions to explicitly compare against the specific data type and value expected for that implementation context.

- Leverage TypeScript type information to make these explicit comparisons precise. For example:
- Redundant checks for conditions already guaranteed by TypeScript type definitions (e.g., checking for `null` when a type is `SomeType | undefined`, or checking for `undefined` on a non-optional, initialized variable) MUST be omitted.
- If a string type is a union of specific non-empty literals (e.g., `"active" | "pending"`), `typeof variable === "string"` can be sufficient for its "truthiness" if all literals are inherently truthy.
- If types and initialization guarantee a variable is always defined and non-null (e.g., a non-optional class member), runtime checks for its mere existence can be redundant; the explicit boolean should reflect this guarantee.

YOU MUST NEVER replace `if (myVar)` with `if (Boolean(myVar))` or `if (!!myVar)` with `if (Boolean(myVar))`.

This rule's purpose is to encourage a thoughtful evaluation of what "truthy" or "falsy" means for the specific variable and logic.

### Examples:

## Incorrect (MUST NOT DO THIS):

```typescript
// Implicit coercion (flagged by ESLint)
if (someStringOrNull) {
// ...
}

// Explicit coercion using Boolean() (not the preferred fix here)
if (Boolean(someStringOrNull)) {
// ...
}
```

## Correct (Preferred Approach):

- Checking for a non-empty string:

```typescript
if (typeof someStringOrNull === "string" && someStringOrNull != "") {
// ...
}
// Or, if an empty string is valid but null/undefined is not:
if (typeof someStringOrNull === "string") {
// ...
}
```

- Checking against `null` or `undefined`:

```typescript
if (someValue !== null && someValue !== undefined) {
// ...
}
// Shorter (catches both null and undefined, mind other falsy values):
if (someValue != null) {
// ...
}
```

- Checking if a variable is assigned (not `undefined`):

```typescript
if (someOptionalValue !== undefined) {
// ...
}
```

- Checking if an array has elements:
```typescript
if (Array.isArray(myArray) && myArray.length > 0) {
// ...
}
```

### Rationale:

Explicitly comparing types and values:

1. Clarifies the code's intent.
2. Reduces ambiguity regarding how falsy values (`null`, `undefined`, `0`, `""`, `NaN`) are handled.
3. Helps avoid bugs from overly general truthiness checks when specific conditions were needed.
4. Be careful of regular expression evaluations. For example, `/foo (.*)bar/` will legitimately match an empty string, or it may not match at all. You MUST differentiate between `match === undefined` vs `typeof match === 'string' && match != ""` because falsy evaluation MUST NOT BE USED because it is usually invalid and certainly imprecise.
5. Helps in distinguishing the original intent behind a `!!variable` check: was it for general "truthiness" or mere "existence (not undefined/null)"? This complements the type-aware checks mentioned in the Guideline.

Always consider the context: What does "truthy" or "falsy" mean for this variable in this logic? Write conditions reflecting that precise meaning.
63 changes: 63 additions & 0 deletions .roo/rules/eslint-no-unused-vars.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Handling `@typescript-eslint/no-unused-vars`

If `@typescript-eslint/no-unused-vars` flags an error, a declaration is unused.

## Guideline: Omit or Delete

Unused declarations MUST be omitted or deleted.

CRITICAL: Unused declarations MUST NOT be "fixed" by prefixing with an underscore (`_`). This is disallowed. Remove dead code, do not silence the linter.

### Examples:

#### Incorrect:

```typescript
// error: 'unusedVar' is defined but never used.
function example(usedParam: string, unusedParam: number): void {
console.log(usedParam)
// Incorrect fix:
// const _unusedVar = 10;
}
```

```typescript
// error: 'error' is defined but never used.
try {
// ...
} catch (error) {
// 'error' is unused
// Incorrect fix:
// } catch (_error) {
console.error("An operation failed.")
}
```

#### Correct:

```typescript
// 'unusedParam' removed if not needed by an interface/override.
function example(usedParam: string): void {
// 'unusedParam' removed
console.log(usedParam)
// 'unusedVar' is completely removed.
}
```

```typescript
// 'error' variable is removed from the catch block if not used.
try {
// ...
} catch {
// 'error' variable omitted entirely
console.error("An operation failed.")
}
```

### Rationale:

- Clarity: Removing unused code improves readability.
- Bugs: Unused variables can indicate errors.
- No Workarounds: Prefixing with `_` hides issues.

Code should be actively used.
2 changes: 2 additions & 0 deletions src/api/providers/__tests__/openai.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ describe("OpenAiHandler", () => {
const stream = reasoningHandler.createMessage(systemPrompt, messages)
// Consume the stream to trigger the API call
for await (const _chunk of stream) {
console.debug("Consuming stream for reasoning test")
}
// Assert the mockCreate was called with reasoning_effort
expect(mockCreate).toHaveBeenCalled()
Expand All @@ -191,6 +192,7 @@ describe("OpenAiHandler", () => {
const stream = noReasoningHandler.createMessage(systemPrompt, messages)
// Consume the stream to trigger the API call
for await (const _chunk of stream) {
console.debug("Consuming stream for no-reasoning test")
}
// Assert the mockCreate was called without reasoning_effort
expect(mockCreate).toHaveBeenCalled()
Expand Down
2 changes: 1 addition & 1 deletion src/api/providers/vscode-lm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export class VsCodeLmHandler extends BaseProvider implements SingleCompletionHan
if (!this.client) {
console.debug("Roo Code <Language Model API>: Getting client with options:", {
vsCodeLmModelSelector: this.options.vsCodeLmModelSelector,
hasOptions: !!this.options,
hasOptions: true, // this.options is guaranteed to be an ApiHandlerOptions object
selectorKeys: this.options.vsCodeLmModelSelector ? Object.keys(this.options.vsCodeLmModelSelector) : [],
})

Expand Down
4 changes: 2 additions & 2 deletions src/core/condense/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ export async function summarizeConversation(
TelemetryService.instance.captureContextCondensed(
taskId,
isAutomaticTrigger ?? false,
!!customCondensingPrompt?.trim(),
!!condensingApiHandler,
typeof customCondensingPrompt === "string" && customCondensingPrompt.trim() !== "",
condensingApiHandler !== undefined,
)

const response: SummarizeResponse = { messages, cost: 0, summary: "" }
Expand Down
2 changes: 1 addition & 1 deletion src/core/config/ContextProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export class ContextProxy {

await this.setValues({
...PROVIDER_SETTINGS_KEYS.filter((key) => !isSecretStateKey(key))
.filter((key) => !!this.stateCache[key])
.filter((key) => this.stateCache[key] != null)
.reduce((acc, key) => ({ ...acc, [key]: undefined }), {} as ProviderSettings),
...values,
})
Expand Down
4 changes: 3 additions & 1 deletion src/core/config/importExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,5 +117,7 @@ export const exportSettings = async ({ providerSettingsManager, contextProxy }:
const dirname = path.dirname(uri.fsPath)
await fs.mkdir(dirname, { recursive: true })
await fs.writeFile(uri.fsPath, JSON.stringify({ providerProfiles, globalSettings }, null, 2), "utf-8")
} catch (e) {}
} catch (e) {
console.debug("Error exporting settings:", e)
}
}
11 changes: 9 additions & 2 deletions src/core/tools/executeCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ export async function executeCommand(
message = { text, images }
process.continue()
}
} catch (_error) {}
} catch (_error) {
console.debug("Error in onWillRespond:", _error)
}
},
onCompleted: (output: string | undefined) => {
result = Terminal.compressTerminalOutput(output ?? "", terminalOutputLineLimit)
Expand All @@ -197,7 +199,12 @@ export async function executeCommand(
}
}

const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, !!customCwd, cline.taskId, terminalProvider)
const terminal = await TerminalRegistry.getOrCreateTerminal(
workingDir,
typeof customCwd === "string" && customCwd !== "",
cline.taskId,
terminalProvider,
)

if (terminal instanceof Terminal) {
terminal.terminal.show(true)
Expand Down
2 changes: 1 addition & 1 deletion src/core/tools/writeToFileTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export async function writeToFileTool(
const isNewFile = !fileExists

// Check if diffStrategy is enabled
const diffStrategyEnabled = !!cline.diffStrategy
const diffStrategyEnabled = cline.diffStrategy !== undefined

// Use more specific error message for line_count that provides guidance based on the situation
await cline.say(
Expand Down
4 changes: 2 additions & 2 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ export class ClineProvider
}

public hasProviderProfileEntry(name: string): boolean {
return !!this.getProviderProfileEntry(name)
return this.getProviderProfileEntry(name) !== undefined
}

async upsertProviderProfile(
Expand Down Expand Up @@ -1656,7 +1656,7 @@ export class ClineProvider
apiProvider: apiConfiguration?.apiProvider,
modelId: task?.api?.getModel().id,
diffStrategy: task?.diffStrategy?.getName(),
isSubtask: task ? !!task.parentTask : undefined,
isSubtask: task ? task.parentTask !== undefined : undefined,
}
}
}
7 changes: 5 additions & 2 deletions src/core/webview/webviewMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We
// Send the result back to the webview
await provider.postMessageToWebview({
type: "browserConnectionResult",
success: !!chromeHostUrl,
success: typeof chromeHostUrl === "string" && chromeHostUrl !== "",
text: `Auto-discovered and tested connection to Chrome: ${chromeHostUrl}`,
values: { endpoint: chromeHostUrl },
})
Expand Down Expand Up @@ -1008,7 +1008,10 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We
// Try to get enhancement config first, fall back to current config.
let configToUse: ProviderSettings = apiConfiguration

if (enhancementApiConfigId && !!listApiConfigMeta.find(({ id }) => id === enhancementApiConfigId)) {
if (
enhancementApiConfigId &&
listApiConfigMeta.find(({ id }) => id === enhancementApiConfigId) !== undefined
) {
const { name: _, ...providerSettings } = await provider.providerSettingsManager.getProfile({
id: enhancementApiConfigId,
})
Expand Down
5 changes: 4 additions & 1 deletion src/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ export default [
// TODO: These should be fixed and the rules re-enabled.
"no-regex-spaces": "off",
"no-useless-escape": "off",
"no-empty": "off",
"prefer-const": "off",

"no-empty": "error",
"no-implicit-coercion": "error",
"no-useless-catch": "error",

"@typescript-eslint/no-unused-vars": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-require-imports": "off",
Expand Down
4 changes: 3 additions & 1 deletion src/integrations/misc/open-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ export async function openFile(filePath: string, options: OpenFileOptions = {})
break
}
}
} catch {} // not essential, sometimes tab operations fail
} catch (e) {
console.debug("Error processing tab operations:", e)
} // not essential, sometimes tab operations fail

const document = await vscode.workspace.openTextDocument(uriToProcess)
const selection =
Expand Down
4 changes: 3 additions & 1 deletion src/integrations/terminal/ExecaTerminalProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
timeoutId = setTimeout(() => {
try {
subprocess.kill("SIGKILL")
} catch (e) {}
} catch (e) {
console.debug("Error killing subprocess:", e)
}

resolve()
}, 5_000)
Expand Down
2 changes: 1 addition & 1 deletion src/services/checkpoints/ShadowCheckpointService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
}

public get isInitialized() {
return !!this.git
return this.git !== undefined
}

constructor(taskId: string, checkpointsDir: string, workspaceDir: string, log: (message: string) => void) {
Expand Down
4 changes: 3 additions & 1 deletion src/services/checkpoints/excludes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,9 @@ const getLfsPatterns = async (workspacePath: string) => {
.filter((line) => line.includes("filter=lfs"))
.map((line) => line.split(" ")[0].trim())
}
} catch (error) {}
} catch (error) {
console.debug("Error getting LFS tracked files:", error)
}

return []
}
Expand Down
9 changes: 7 additions & 2 deletions src/services/code-index/config-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,18 @@ export class CodeIndexConfigManager {
if (this.embedderProvider === "openai") {
const openAiKey = this.openAiOptions?.openAiNativeApiKey
const qdrantUrl = this.qdrantUrl
const isConfigured = !!(openAiKey && qdrantUrl)
const isConfigured =
typeof openAiKey === "string" && openAiKey !== "" && typeof qdrantUrl === "string" && qdrantUrl !== ""
return isConfigured
} else if (this.embedderProvider === "ollama") {
// Ollama model ID has a default, so only base URL is strictly required for config
const ollamaBaseUrl = this.ollamaOptions?.ollamaBaseUrl
const qdrantUrl = this.qdrantUrl
const isConfigured = !!(ollamaBaseUrl && qdrantUrl)
const isConfigured =
typeof ollamaBaseUrl === "string" &&
ollamaBaseUrl !== "" &&
typeof qdrantUrl === "string" &&
qdrantUrl !== ""
return isConfigured
}
return false // Should not happen if embedderProvider is always set correctly
Expand Down
Loading