Skip to content

Commit 489c2cb

Browse files
fix(pipeline): avoid .github false negatives; analyze package-lock only with lockfile rules; bump module analysis cache schema
1 parent 1d1dd64 commit 489c2cb

4 files changed

Lines changed: 139 additions & 5 deletions

File tree

pipeline/workers/process-module.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,11 @@ async function analyzeModule(module: ModuleInput, config: ProcessModuleConfig):
829829
const fullPath = path.join(dir, entry.name);
830830
const relativePath = path.relative(baseDir, fullPath);
831831

832-
if (!relativePath.includes(".git") && !relativePath.includes("node_modules")) {
832+
const pathSegments = relativePath.split(path.sep);
833+
const isGitDir = pathSegments.includes(".git");
834+
const isNodeModulesDir = pathSegments.includes("node_modules");
835+
836+
if (!isGitDir && !isNodeModulesDir) {
833837
if (entry.isDirectory()) {
834838
getAllFiles(fullPath, baseDir, files);
835839
} else {
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import * as fsPromises from "node:fs/promises";
2+
import { analyzeModule } from "../../check-modules/module-analyzer.ts";
3+
import { strict as assert } from "node:assert";
4+
import { join } from "node:path";
5+
import { test } from "node:test";
6+
import { tmpdir } from "node:os";
7+
8+
async function createTempModule() {
9+
const root = await fsPromises.mkdtemp(join(tmpdir(), "module-analyzer-test-"));
10+
await fsPromises.mkdir(join(root, ".github"), { recursive: true });
11+
12+
await fsPromises.writeFile(
13+
join(root, "README.md"),
14+
[
15+
"# MMM-Remote-Control",
16+
"",
17+
"## Installation",
18+
"git clone https://github.com/Jopyth/MMM-Remote-Control",
19+
"",
20+
"## Update",
21+
"Update steps",
22+
"",
23+
"## Config",
24+
"{",
25+
" module: \"MMM-Remote-Control\",",
26+
" config: {",
27+
" foo: true",
28+
" },",
29+
"},",
30+
""
31+
].join("\n")
32+
);
33+
34+
await fsPromises.writeFile(
35+
join(root, "CHANGELOG.md"),
36+
"XMLHttpRequest\nnpm run\ngit checkout\n"
37+
);
38+
await fsPromises.writeFile(join(root, "CODE_OF_CONDUCT.md"), "code of conduct");
39+
await fsPromises.writeFile(join(root, "LICENSE.md"), "license");
40+
await fsPromises.writeFile(join(root, "eslint.config.mjs"), "export default [];\n");
41+
await fsPromises.writeFile(
42+
join(root, "package.json"),
43+
JSON.stringify(
44+
{
45+
devDependencies: { eslint: "^10.0.0" },
46+
scripts: { lint: "eslint" }
47+
},
48+
null,
49+
2
50+
)
51+
);
52+
await fsPromises.writeFile(
53+
join(root, "package-lock.json"),
54+
`${JSON.stringify(
55+
{
56+
name: "x",
57+
note: "jshint",
58+
lockfileVersion: 2,
59+
scripts: { test: "npm run lint" }
60+
},
61+
null,
62+
2
63+
)}\n`
64+
);
65+
await fsPromises.writeFile(join(root, ".github", "dependabot.yaml"), "version: 2\nupdates: []\n");
66+
67+
return root;
68+
}
69+
70+
test("analyzer keeps .github files and applies only lockfile-specific package-lock rules", async () => {
71+
const moduleRoot = await createTempModule();
72+
const files = [
73+
join(moduleRoot, "README.md"),
74+
join(moduleRoot, "CHANGELOG.md"),
75+
join(moduleRoot, "CODE_OF_CONDUCT.md"),
76+
join(moduleRoot, "LICENSE.md"),
77+
join(moduleRoot, "eslint.config.mjs"),
78+
join(moduleRoot, "package.json"),
79+
join(moduleRoot, "package-lock.json"),
80+
join(moduleRoot, ".github", "dependabot.yaml")
81+
];
82+
83+
const result = await analyzeModule(
84+
moduleRoot,
85+
"MMM-Remote-Control",
86+
"https://github.com/Jopyth/MMM-Remote-Control",
87+
files
88+
);
89+
90+
assert.equal(
91+
result.issues.some(issue => issue.includes("There is no dependabot configuration file")),
92+
false
93+
);
94+
assert.equal(
95+
result.issues.some(issue => issue.includes("in file `CHANGELOG.md`")),
96+
false
97+
);
98+
assert.equal(
99+
result.issues.some(
100+
issue => issue.includes("Found `jshint`") && issue.includes("in file `package-lock.json`")
101+
),
102+
false
103+
);
104+
assert.equal(
105+
result.issues.some(
106+
issue => issue.includes("Found `\"lockfileVersion\": 2`") && issue.includes("in file `package-lock.json`")
107+
),
108+
true
109+
);
110+
111+
assert.ok(Array.isArray(result.issues));
112+
});

scripts/check-modules/module-analyzer.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,17 +331,35 @@ export async function analyzeModule(
331331
): Promise<AnalysisResult> {
332332
const issues: string[] = [];
333333

334-
// Filter out files in node_modules and .git
334+
// Filter out files in node_modules and .git directories.
335+
// Use path segments instead of substring matching so ".github" files are not excluded.
335336
const relevantFiles = files.filter(
336-
(f) => !f.includes("node_modules") && !f.includes(".git")
337+
(f) => {
338+
const segments = f.split("/");
339+
return !segments.includes("node_modules") && !segments.includes(".git");
340+
}
337341
);
338342

339343
// Check for each file
340344
for (const filePath of relevantFiles) {
341345
const content = await readFile(filePath, "utf-8").catch(() => "");
346+
const filename = filePath.split("/").pop() ?? "";
347+
const filenameLower = filename.toLowerCase();
348+
const isChangelogFile = filenameLower === "changelog" || filenameLower.startsWith("changelog.");
349+
const isPackageLockFile = filenameLower === "package-lock.json";
342350

343351
// Check TEXT_RULES
344352
for (const [, rule] of Object.entries(TEXT_RULES)) {
353+
// CHANGELOG entries are historical context and produce low-quality findings.
354+
if (isChangelogFile) {
355+
continue;
356+
}
357+
358+
// lockfiles should only be checked via lockfile-specific rules.
359+
if (isPackageLockFile) {
360+
continue;
361+
}
362+
345363
if (content.includes(rule.pattern)) {
346364
issues.push(
347365
`${rule.category}: Found \`${rule.pattern}\` in file \`${filePath.split("/").pop()}\`: ${rule.description}`
@@ -367,7 +385,7 @@ export async function analyzeModule(
367385
}
368386

369387
// Package-lock.json rules
370-
if (filePath.endsWith("package-lock.json")) {
388+
if (isPackageLockFile) {
371389
for (const [, rule] of Object.entries(PACKAGE_LOCK_RULES)) {
372390
if (content.includes(rule.pattern)) {
373391
issues.push(

scripts/shared/module-analysis-cache.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { getCurrentCommit } from "./git.ts";
33
import { resolve } from "node:path";
44
import { stringifyDeterministic } from "./deterministic-output.ts";
55

6-
export const MODULE_ANALYSIS_CACHE_SCHEMA_VERSION = 4;
6+
export const MODULE_ANALYSIS_CACHE_SCHEMA_VERSION = 6;
77
export const MODULE_ANALYSIS_CACHE_RELATIVE_PATH = "website/data/moduleCache.json";
88

99
interface ModuleAnalysisCheckGroupsInput {

0 commit comments

Comments
 (0)