Skip to content

Commit

Permalink
JS-493 Add 'get-telemetry' endpoint on bridge-server (#5011)
Browse files Browse the repository at this point in the history
Co-authored-by: Ilia Kebets <ilia.kebets@sonarsource.com>
  • Loading branch information
zglicz and kebetsi authored Dec 12, 2024
1 parent 476c0d9 commit 3a5d10a
Show file tree
Hide file tree
Showing 17 changed files with 323 additions and 23 deletions.
6 changes: 5 additions & 1 deletion packages/bridge/src/handle-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
import { analyzeCSS } from '../../css/src/analysis/analyzer.js';
import { analyzeHTML } from '../../html/src/index.js';
import { analyzeJSTS } from '../../jsts/src/analysis/analyzer.js';
import { analyzeJSTS, getTelemetry } from '../../jsts/src/analysis/analyzer.js';
import { analyzeProject } from '../../jsts/src/analysis/projectAnalysis/projectAnalyzer.js';
import { analyzeYAML } from '../../yaml/src/index.js';
import { logHeapStatistics } from './memory.js';
Expand Down Expand Up @@ -107,6 +107,10 @@ export async function handleRequest(request: BridgeRequest): Promise<RequestResu
const output = await analyzeProject(request.data);
return { type: 'success', result: output };
}
case 'on-get-telemetry': {
const output = getTelemetry();
return { type: 'success', result: output };
}
}
} catch (err) {
return { type: 'failure', error: serializeError(err) };
Expand Down
13 changes: 11 additions & 2 deletions packages/bridge/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,22 @@ import { TsConfigJson } from 'type-fest';
import { RuleConfig } from '../../jsts/src/linter/config/rule-config.js';
import { readFile } from '../../shared/src/helpers/files.js';
import { APIError, ErrorCode } from '../../shared/src/errors/error.js';
import { NamedDependency } from '../../jsts/src/rules/index.js';

export type RequestResult =
| {
type: 'success';
result: string | AnalysisOutput;
result: string | AnalysisOutput | Telemetry;
}
| {
type: 'failure';
error: ReturnType<typeof serializeError>;
};

export type Telemetry = {
dependencies: NamedDependency[];
};

export type RequestType = BridgeRequest['type'];

type MaybeIncompleteCssAnalysisInput = Omit<CssAnalysisInput, 'fileContent'> & {
Expand Down Expand Up @@ -61,7 +66,8 @@ export type BridgeRequest =
| DeleteProgramRequest
| InitLinterRequest
| NewTsConfigRequest
| TsConfigFilesRequest;
| TsConfigFilesRequest
| GetTelemetryRequest;

type CssRequest = {
type: 'on-analyze-css';
Expand Down Expand Up @@ -115,6 +121,9 @@ type TsConfigFilesRequest = {
type: 'on-tsconfig-files';
data: { tsConfig: string };
};
type GetTelemetryRequest = {
type: 'on-get-telemetry';
};

/**
* In SonarQube context, an analysis input includes both path and content of a file
Expand Down
1 change: 1 addition & 0 deletions packages/bridge/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export default function (worker?: Worker): express.Router {
router.post('/init-linter', delegate('on-init-linter'));
router.post('/new-tsconfig', delegate('on-new-tsconfig'));
router.post('/tsconfig-files', delegate('on-tsconfig-files'));
router.get('/get-telemetry', delegate('on-get-telemetry'));

/** Endpoints running on the main thread */
router.get('/status', (_, response) => response.send('OK!'));
Expand Down
6 changes: 6 additions & 0 deletions packages/bridge/tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ describe('router', () => {
expect(json.filename).toBeTruthy();
expect(fs.existsSync(json.filename)).toBe(true);
});

it('should return empty get-telemetry on fresh server', async () => {
const response = (await request(server, '/get-telemetry', 'GET')) as string;
const json = JSON.parse(response);
expect(json).toEqual({ dependencies: [] });
});
});

function requestInitLinter(server: http.Server, rules: RuleConfig[]) {
Expand Down
9 changes: 8 additions & 1 deletion packages/jsts/src/analysis/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import { getContext } from '../../../shared/src/helpers/context.js';
import { computeMetrics, findNoSonarLines } from '../linter/visitors/metrics/index.js';
import { getSyntaxHighlighting } from '../linter/visitors/syntax-highlighting.js';
import { getCpdTokens } from '../linter/visitors/cpd.js';
import { clearDependenciesCache } from '../rules/helpers/package-json.js';
import { clearDependenciesCache, getAllDependencies } from '../rules/index.js';
import { Telemetry } from '../../../bridge/src/request.js';

/**
* Analyzes a JavaScript / TypeScript analysis input
Expand Down Expand Up @@ -160,3 +161,9 @@ function computeExtendedMetrics(
};
}
}

export function getTelemetry(): Telemetry {
return {
dependencies: getAllDependencies(),
};
}
73 changes: 56 additions & 17 deletions packages/jsts/src/rules/helpers/package-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,42 @@ const findPackageJsons = createFindUp(PACKAGE_JSON);

const DefinitelyTyped = '@types/';

type MinimatchDependency = {
name: Minimatch;
version?: string;
};

export type NamedDependency = {
name: string;
version?: string;
};

type Dependency = MinimatchDependency | NamedDependency;

/**
* Cache for the available dependencies by dirname.
*/
const cache: Map<string, Set<string | Minimatch>> = new Map();
const cache: Map<string, Set<Dependency>> = new Map();

/**
* Returns the dependencies of the root package.json file collected in the cache.
* As the cache is populated lazily, it could be null in case no rule execution has touched it.
* This removes duplicate dependencies and keeps the last occurrence.
*/
export function getAllDependencies(): NamedDependency[] {
const dependencies = [...cache.values()]
.flatMap(dependencies => [...dependencies])
.filter((dependency): dependency is NamedDependency => typeof dependency.name === 'string');
return Object.values(
dependencies.reduce(
(result, dependency) => ({
...result,
[dependency.name]: dependency,
}),
{},
),
);
}

/**
* Retrieve the dependencies of all the package.json files available for the given file.
Expand All @@ -46,9 +78,9 @@ export function getDependencies(filename: string, cwd: string) {
const dirname = Path.dirname(toUnixPath(filename));
const cached = cache.get(dirname);
if (cached) {
return cached;
return new Set([...cached].map(item => item.name));
}
const result = new Set<string | Minimatch>();
const result = new Set<Dependency>();
cache.set(dirname, result);

getManifests(dirname, cwd, fs).forEach(manifest => {
Expand All @@ -59,7 +91,7 @@ export function getDependencies(filename: string, cwd: string) {
});
});

return result;
return new Set([...result].map(item => item.name));
}

/**
Expand All @@ -71,7 +103,7 @@ export function clearDependenciesCache() {
}

export function getDependenciesFromPackageJson(content: PackageJson) {
const result = new Set<string | Minimatch>();
const result = new Set<Dependency>();
if (content.name) {
addDependencies(result, { [content.name]: '*' });
}
Expand Down Expand Up @@ -99,30 +131,37 @@ export function getDependenciesFromPackageJson(content: PackageJson) {
}

function addDependencies(
result: Set<string | Minimatch>,
result: Set<Dependency>,
dependencies: PackageJson.Dependency,
isGlob = false,
) {
Object.keys(dependencies).forEach(name => addDependency(result, name, isGlob));
Object.keys(dependencies).forEach(name =>
addDependency(result, name, isGlob, dependencies[name]),
);
}

function addDependenciesArray(
result: Set<string | Minimatch>,
dependencies: string[],
isGlob = true,
) {
function addDependenciesArray(result: Set<Dependency>, dependencies: string[], isGlob = true) {
dependencies.forEach(name => addDependency(result, name, isGlob));
}

function addDependency(result: Set<string | Minimatch>, dependency: string, isGlob: boolean) {
function addDependency(
result: Set<Dependency>,
dependency: string,
isGlob: boolean,
version?: string,
) {
if (isGlob) {
result.add(new Minimatch(dependency, { nocase: true, matchBase: true }));
result.add({
name: new Minimatch(dependency, { nocase: true, matchBase: true }),
version,
});
} else {
result.add(
dependency.startsWith(DefinitelyTyped)
result.add({
name: dependency.startsWith(DefinitelyTyped)
? dependency.substring(DefinitelyTyped.length)
: dependency,
);
version,
});
}
}

Expand Down
47 changes: 45 additions & 2 deletions packages/jsts/tests/analysis/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import { jsTsInput, parseJavaScriptSourceFile } from '../tools/index.js';
import { Linter, Rule } from 'eslint';
import { describe, beforeEach, it } from 'node:test';
import { expect } from 'expect';
import { getManifests, toUnixPath } from '../../src/rules/helpers/index.js';
import { getDependencies, getManifests, toUnixPath } from '../../src/rules/helpers/index.js';
import { setContext } from '../../../shared/src/helpers/context.js';
import { analyzeJSTS } from '../../src/analysis/analyzer.js';
import { analyzeJSTS, getTelemetry } from '../../src/analysis/analyzer.js';
import { APIError } from '../../../shared/src/errors/error.js';
import { RuleConfig } from '../../src/linter/config/rule-config.js';
import { initializeLinter } from '../../src/linter/linters.js';
Expand Down Expand Up @@ -899,6 +899,49 @@ describe('analyzeJSTS', () => {
expect(vueIssues[0].message).toEqual('call');
});

it('should populate dependencies after analysis', async () => {
const baseDir = path.join(currentPath, 'fixtures', 'dependencies');
const linter = new Linter();
linter.defineRule('custom-rule-file', {
create(context) {
return {
CallExpression(node) {
// Necessarily call 'getDependencies' to populate the cache of dependencies
const dependencies = getDependencies(toUnixPath(context.filename), baseDir);
if (dependencies.size) {
context.report({
node: node.callee,
message: 'call',
});
}
},
};
},
} as Rule.RuleModule);
const filePath = path.join(currentPath, 'fixtures', 'dependencies', 'index.js');
const sourceCode = await parseJavaScriptSourceFile(filePath);
linter.verify(
sourceCode,
{ rules: { 'custom-rule-file': 'error' } },
{ filename: filePath, allowInlineConfig: false },
);
const { dependencies } = getTelemetry();
expect(dependencies).toStrictEqual([
{
name: 'test-module',
version: '*',
},
{
name: 'pkg1',
version: '1.0.0',
},
{
name: 'pkg2',
version: '2.0.0',
},
]);
});

it('should return the AST along with the issues', async () => {
const rules = [{ key: 'S4524', configurations: [], fileTypeTarget: ['MAIN'] }] as RuleConfig[];
await initializeLinter(rules);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("Hello World!");
11 changes: 11 additions & 0 deletions packages/jsts/tests/analysis/fixtures/dependencies/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "test-module",
"version": "1.0.0",
"author": "Your Name <email@example.com>",
"dependencies": {
"pkg1": "1.0.0"
},
"devDependencies": {
"pkg2": "2.0.0"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ void initLinter(

TsConfigFile createTsConfigFile(String content) throws IOException;

TelemetryResponse getTelemetry();

record JsAnalysisRequest(
String filePath,
String fileType,
Expand Down Expand Up @@ -275,4 +277,8 @@ public String toString() {
}

record TsProgramRequest(String tsConfig) {}

record TelemetryResponse(List<Dependency> dependencies) {}

record Dependency(String name, String version) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,16 @@ public TsConfigFile createTsConfigFile(String content) {
return GSON.fromJson(response.json(), TsConfigFile.class);
}

@Override
public TelemetryResponse getTelemetry() {
try {
var result = http.get(url("get-telemetry"));
return GSON.fromJson(result, TelemetryResponse.class);
} catch (IOException e) {
return new TelemetryResponse(List.of());
}
}

private static <T> List<T> emptyListIfNull(@Nullable List<T> list) {
return list == null ? emptyList() : list;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@
import org.sonar.api.utils.TempFolder;
import org.sonar.api.utils.Version;
import org.sonar.plugins.javascript.bridge.BridgeServer.CssAnalysisRequest;
import org.sonar.plugins.javascript.bridge.BridgeServer.Dependency;
import org.sonar.plugins.javascript.bridge.BridgeServer.JsAnalysisRequest;
import org.sonar.plugins.javascript.bridge.BridgeServer.TelemetryResponse;
import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgram;
import org.sonar.plugins.javascript.bridge.BridgeServer.TsProgramRequest;
import org.sonar.plugins.javascript.bridge.protobuf.Node;
Expand Down Expand Up @@ -752,6 +754,16 @@ void test_ucfg_bundle_version() throws Exception {
);
}

@Test
void should_return_telemetry() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.startServer(serverConfig, emptyList());
var telemetry = bridgeServer.getTelemetry();
assertThat(telemetry).isEqualTo(
new TelemetryResponse(List.of(new Dependency("pkg1", "1.0.0")))
);
}

@Test
void should_return_an_ast() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ const requestHandler = (request, response) => {
loc: {}}]}]}],
highlights: [{location: {startLine: 0, startColumn: 0, endLine: 0, endColumn: 0}}],
metrics: {}, highlightedSymbols: [{}], cpdTokens: [{}] }`);
} else if (request.url === '/get-telemetry') {
response.end('{"dependencies": [{"name": "pkg1", "version": "1.0.0"}]}');
} else {
// /analyze-with-program
// /analyze-js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ void analyzeFiles(List<InputFile> inputFiles) throws IOException {
)
);
}
var telemetry = bridgeServer.getTelemetry();
new PluginTelemetry(context).reportTelemetry(telemetry);
} finally {
if (success) {
progressReport.stop();
Expand Down
Loading

0 comments on commit 3a5d10a

Please sign in to comment.