Skip to content

Commit 89a836e

Browse files
bajrangCoderbajrangCoder
authored
fix(editor): prevent false language mode recommendations (Acode-Foundation#2621)
* fix(editor): prevent false language mode recommendations - recommend only language-mode plugins available in the registry - suppress built-in, extensionless, and arbitrary file false positives - remove the direct plugin-request issue action - add regression tests for unknown extensions * fix the stale network cache things
1 parent a9e7a1a commit 89a836e

3 files changed

Lines changed: 174 additions & 60 deletions

File tree

‎src/lib/languageModeRecommendations.js‎

Lines changed: 47 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -23,32 +23,12 @@ export function getLanguageModeRecommendationSearchKeyword(filename) {
2323
return keyword;
2424
}
2525

26-
function getIssueUrl(keyword) {
27-
const params = new URLSearchParams({
28-
template: "1_feature_request.yml",
29-
labels: "new plugin idea,enhancement",
30-
title: `Plugin request: ${keyword} syntax highlighting`,
31-
});
32-
33-
return `${config.GITHUB_URL}/issues/new?${params}`;
34-
}
35-
3626
function formatString(value, replacements) {
3727
return String(value || "").replace(/\{(\w+)\}/g, (_, key) => {
3828
return replacements[key] ?? "";
3929
});
4030
}
4131

42-
async function openUrl(url) {
43-
if (window.cordova?.exec) {
44-
const { default: customTab } = await import("./customTab");
45-
await customTab(url);
46-
return;
47-
}
48-
49-
window.open(url, "_blank", "noopener,noreferrer");
50-
}
51-
5232
async function openExtensions(keyword) {
5333
const { openWithSearch } = await import("sidebarApps/extensions");
5434
openWithSearch(keyword);
@@ -58,6 +38,19 @@ function hasPlainTextFallback(modeInfo, filename) {
5838
return modeInfo?.name === "text" && !modeInfo.supportsFile(filename);
5939
}
6040

41+
export function shouldRecommendLanguageModeExtension(filename, modeInfo) {
42+
if (!hasPlainTextFallback(modeInfo, filename)) return false;
43+
44+
const keyword = getLanguageModeRecommendationSearchKeyword(filename);
45+
if (!keyword) return false;
46+
47+
// Probe the normalized extension independently of the original filename.
48+
// This prevents a strangely formatted path from producing requests for core
49+
// modes such as HTML or Python.
50+
const probeFilename = `file.${keyword}`;
51+
return hasPlainTextFallback(getModeForPath(probeFilename), probeFilename);
52+
}
53+
6154
class LanguageModeRecommendations {
6255
notifiedKeywords = new Set();
6356
pendingKeywords = new Set();
@@ -76,9 +69,21 @@ class LanguageModeRecommendations {
7669
),
7770
),
7871
)
79-
.then((response) => (response.ok ? response.json() : []))
72+
.then((response) => {
73+
if (!response.ok) {
74+
throw new Error(`Plugin registry request failed: ${response.status}`);
75+
}
76+
return response.json();
77+
})
8078
.then((plugins) => Array.isArray(plugins) && plugins.length > 0)
81-
.catch(() => false);
79+
.catch(() => {
80+
// Do not let a temporary network or server failure suppress this
81+
// recommendation for the rest of the app session.
82+
if (this.availabilityCache.get(keyword) === availability) {
83+
this.availabilityCache.delete(keyword);
84+
}
85+
return false;
86+
});
8287

8388
this.availabilityCache.set(keyword, availability);
8489
return availability;
@@ -88,7 +93,7 @@ class LanguageModeRecommendations {
8893
if (!file || file.type !== "editor") return;
8994

9095
const filename = file.filename || "";
91-
if (!hasPlainTextFallback(modeInfo, filename)) return;
96+
if (!shouldRecommendLanguageModeExtension(filename, modeInfo)) return;
9297

9398
const keyword = getLanguageModeRecommendationSearchKeyword(filename);
9499
if (
@@ -116,52 +121,35 @@ class LanguageModeRecommendations {
116121
const hasPlugins = await this.getPluginAvailability(keyword);
117122
// If a plugin registered the mode while the lookup was pending, suppress
118123
// this stale recommendation and leave the keyword eligible for future checks.
119-
if (!hasPlainTextFallback(getModeForPath(filename), filename)) return false;
120-
121-
const displayExt = `.${keyword}`;
122-
123-
if (hasPlugins) {
124-
notificationManager.pushNotification({
125-
title: formatString(strings["extension recommendation title"], {
126-
extension: displayExt,
127-
keyword: `mode:${keyword}`,
128-
}),
129-
message: formatString(strings["extension recommendation message"], {
130-
extension: displayExt,
131-
keyword: `mode:${keyword}`,
132-
}),
133-
icon: "extension",
134-
type: "info",
135-
action: () => openExtensions(`mode:${keyword}`),
136-
actions: [
137-
{
138-
text: strings["search plugins"],
139-
icon: "search",
140-
action: () => openExtensions(`mode:${keyword}`),
141-
},
142-
],
143-
});
144-
return true;
124+
if (
125+
!shouldRecommendLanguageModeExtension(filename, getModeForPath(filename))
126+
) {
127+
return false;
145128
}
146129

147-
const issueUrl = getIssueUrl(keyword);
130+
// An unknown extension is not enough evidence that the file contains a
131+
// programming language. Stay silent unless the registry has a matching
132+
// language-mode plugin to recommend.
133+
if (!hasPlugins) return false;
134+
135+
const displayExt = `.${keyword}`;
148136
notificationManager.pushNotification({
149-
title: formatString(strings["extension request title"], {
137+
title: formatString(strings["extension recommendation title"], {
150138
extension: displayExt,
151-
keyword,
139+
keyword: `mode:${keyword}`,
152140
}),
153-
message: formatString(strings["extension request message"], {
141+
message: formatString(strings["extension recommendation message"], {
154142
extension: displayExt,
155-
keyword,
143+
keyword: `mode:${keyword}`,
156144
}),
157145
icon: "extension",
158-
type: "warning",
159-
action: () => openUrl(issueUrl),
146+
type: "info",
147+
action: () => openExtensions(`mode:${keyword}`),
160148
actions: [
161149
{
162-
text: strings["request plugin"],
163-
icon: "open_in_new",
164-
action: () => openUrl(issueUrl),
150+
text: strings["search plugins"],
151+
icon: "search",
152+
action: () => openExtensions(`mode:${keyword}`),
165153
},
166154
],
167155
});

‎src/test/sanity.tests.js‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1+
import { getModeForPath } from "../cm/modelist";
12
import {
23
clearModifierState,
34
clearQuickToolsButtonFeedback,
45
removeActionStackEntries,
56
} from "../handlers/quickToolsState";
6-
import { getLanguageModeRecommendationSearchKeyword } from "../lib/languageModeRecommendations";
7+
import {
8+
getLanguageModeRecommendationSearchKeyword,
9+
shouldRecommendLanguageModeExtension,
10+
} from "../lib/languageModeRecommendations";
711
import { isVersionGreater } from "../utils/version";
812
import { TestRunner } from "./tester";
913

@@ -86,6 +90,35 @@ export async function runSanityTests(writeOutput) {
8690
"",
8791
"Extensionless non-dotfiles should not request plugin recommendations",
8892
);
93+
test.assertEqual(
94+
getLanguageModeRecommendationSearchKeyword("example"),
95+
"",
96+
"Arbitrary extensionless names should not request plugin recommendations",
97+
);
98+
});
99+
100+
runner.test("Language mode recommendation candidates", (test) => {
101+
test.assert(
102+
!shouldRecommendLanguageModeExtension(
103+
"example.html ",
104+
getModeForPath("example.html "),
105+
),
106+
"Built-in language extensions should not request plugins",
107+
);
108+
test.assert(
109+
!shouldRecommendLanguageModeExtension(
110+
"example.py ",
111+
getModeForPath("example.py "),
112+
),
113+
"Built-in Python support should not request a plugin",
114+
);
115+
test.assert(
116+
shouldRecommendLanguageModeExtension(
117+
"example.acode-unknown-mode",
118+
getModeForPath("example.acode-unknown-mode"),
119+
),
120+
"Unknown language extensions should remain eligible for recommendations",
121+
);
89122
});
90123

91124
runner.test("Quick tools modifier cleanup emits inactive state", (test) => {

‎tests/unit/languageModeRecommendations.test.js‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
2+
import "cm/supportedModes";
3+
import { getModeForPath } from "cm/modelist";
4+
import notificationManager from "lib/notificationManager";
5+
import recommendLanguageModeExtension from "lib/languageModeRecommendations";
6+
7+
vi.mock("lib/notificationManager", () => ({
8+
default: {
9+
pushNotification: vi.fn(),
10+
},
11+
}));
12+
13+
const originalFetch = globalThis.fetch;
14+
15+
globalThis.strings = {
16+
"extension recommendation title": "Extensions available for {extension}",
17+
"extension recommendation message": "Search for {keyword}",
18+
"search plugins": "Search plugins",
19+
};
20+
21+
function recommend(filename) {
22+
recommendLanguageModeExtension(
23+
{ type: "editor", filename },
24+
getModeForPath(filename),
25+
);
26+
}
27+
28+
describe("language mode recommendations", () => {
29+
beforeEach(() => {
30+
vi.clearAllMocks();
31+
});
32+
33+
afterAll(() => {
34+
globalThis.fetch = originalFetch;
35+
});
36+
37+
it("stays silent for arbitrary extensions without a matching plugin", async () => {
38+
globalThis.fetch = vi.fn().mockResolvedValue({
39+
ok: true,
40+
json: async () => [],
41+
});
42+
43+
for (const filename of ["test.random", "dhd.sdocx", "hdh.glsl"]) {
44+
recommend(filename);
45+
}
46+
47+
await vi.waitFor(() => expect(globalThis.fetch).toHaveBeenCalled());
48+
await new Promise((resolve) => setTimeout(resolve, 0));
49+
50+
expect(notificationManager.pushNotification).not.toHaveBeenCalled();
51+
});
52+
53+
it("recommends a language-mode plugin that exists in the registry", async () => {
54+
globalThis.fetch = vi.fn().mockResolvedValue({
55+
ok: true,
56+
json: async () => [{ id: "example-language-mode" }],
57+
});
58+
59+
recommend("test.acodepluginmode");
60+
61+
await vi.waitFor(() => {
62+
expect(notificationManager.pushNotification).toHaveBeenCalledOnce();
63+
});
64+
});
65+
66+
it.each([
67+
["network errors", () => Promise.reject(new Error("offline"))],
68+
["server errors", () => Promise.resolve({ ok: false, status: 503 })],
69+
])("retries after transient %s", async (_, failedResponse) => {
70+
globalThis.fetch = vi
71+
.fn()
72+
.mockImplementationOnce(failedResponse)
73+
.mockResolvedValueOnce({
74+
ok: true,
75+
json: async () => [{ id: "recovered-language-mode" }],
76+
});
77+
78+
const keyword = `retryable-${crypto.randomUUID()}`;
79+
recommend(`test.${keyword}`);
80+
81+
await vi.waitFor(() => {
82+
expect(globalThis.fetch).toHaveBeenCalledOnce();
83+
});
84+
await new Promise((resolve) => setTimeout(resolve, 0));
85+
86+
recommend(`test.${keyword}`);
87+
88+
await vi.waitFor(() => {
89+
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
90+
expect(notificationManager.pushNotification).toHaveBeenCalledOnce();
91+
});
92+
});
93+
});

0 commit comments

Comments
 (0)