Skip to content

Commit

Permalink
Implement package linter (#148496)
Browse files Browse the repository at this point in the history
This PR implements a linter like the TS Project linter, except for
packages in the repo. It does this by extracting the reusable bits from
the TS Project linter and reusing them for the project linter. The only
rule that exists for packages right now is that the "name" in the
package.json file matches the "id" in Kibana.jsonc. The goal is to use a
rule to migrate kibana.json files on the future.

Additionally, a new rule for validating the indentation of tsconfig.json
files was added.

Validating and fixing violations is what has triggered review by so many
teams, but we plan to treat those review requests as notifications of
the changes and not as blockers for merging.

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
Spencer and kibanamachine authored Jan 9, 2023
1 parent 039ed99 commit d6be4a4
Show file tree
Hide file tree
Showing 218 changed files with 3,913 additions and 1,646 deletions.
1 change: 1 addition & 0 deletions .buildkite/scripts/steps/checks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export DISABLE_BOOTSTRAP_VALIDATION=false

.buildkite/scripts/steps/checks/precommit_hook.sh
.buildkite/scripts/steps/checks/ts_projects.sh
.buildkite/scripts/steps/checks/packages.sh
.buildkite/scripts/steps/checks/bazel_packages.sh
.buildkite/scripts/steps/checks/verify_notice.sh
.buildkite/scripts/steps/checks/plugin_list_docs.sh
Expand Down
14 changes: 14 additions & 0 deletions .buildkite/scripts/steps/checks/packages.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash

set -euo pipefail

source .buildkite/scripts/common/util.sh

echo --- Lint packages
cmd="node scripts/lint_packages"
if is_pr && ! is_auto_commit_disabled; then
cmd="$cmd --fix"
fi

eval "$cmd"
check_for_changed_files "$cmd" true
4 changes: 2 additions & 2 deletions .buildkite/scripts/steps/checks/ts_projects.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ set -euo pipefail

source .buildkite/scripts/common/util.sh

echo --- Run TS Project Linter
cmd="node scripts/ts_project_linter"
echo --- Lint TS projects
cmd="node scripts/lint_ts_projects"
if is_pr && ! is_auto_commit_disabled; then
cmd="$cmd --fix"
fi
Expand Down
2 changes: 0 additions & 2 deletions .buildkite/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
{
"extends": "../tsconfig.base.json",
"compilerOptions": {
"incremental": false,
"composite": false,
"outDir": "target/types",
"types": ["node", "mocha"],
"paths": {
Expand Down
38 changes: 22 additions & 16 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@

require('@kbn/babel-register').install();

const Path = require('path');
const Fs = require('fs');

const normalizePath = require('normalize-path');
const { discoverPackageManifestPaths, Jsonc } = require('@kbn/bazel-packages');
const { getPackages } = require('@kbn/repo-packages');
const { REPO_ROOT } = require('@kbn/repo-info');

const APACHE_2_0_LICENSE_HEADER = `
Expand Down Expand Up @@ -124,10 +120,9 @@ const VENN_DIAGRAM_HEADER = `
`;

/** Packages which should not be included within production code. */
const DEV_PACKAGE_DIRS = discoverPackageManifestPaths(REPO_ROOT).flatMap((path) => {
const manifest = Jsonc.parse(Fs.readFileSync(path, 'utf8'));
return !!manifest.devOnly ? normalizePath(Path.relative(REPO_ROOT, Path.dirname(path))) : [];
});
const DEV_PACKAGE_DIRS = getPackages(REPO_ROOT).flatMap((pkg) =>
pkg.isDevOnly ? pkg.normalizedRepoRelativeDir : []
);

/** Directories (at any depth) which include dev-only code. */
const DEV_DIRECTORIES = [
Expand Down Expand Up @@ -1700,13 +1695,6 @@ module.exports = {
},
},

/**
* Prettier disables all conflicting rules, listing as last override so it takes precedence
*/
{
files: ['**/*'],
rules: require('eslint-config-prettier').rules,
},
/**
* Enterprise Search Prettier override
* Lints unnecessary backticks - @see https://github.com/prettier/eslint-config-prettier/blob/main/README.md#forbid-unnecessary-backticks
Expand All @@ -1728,5 +1716,23 @@ module.exports = {
'@kbn/imports/no_unresolvable_imports': 'off',
},
},

/**
* Code inside .buildkite runs separately from everything else in CI, before bootstrap, with ts-node. It needs a few tweaks because of this.
*/
{
files: 'packages/kbn-{package-*,repo-*,dep-*}/**/*',
rules: {
'max-classes-per-file': 'off',
},
},
],
};

/**
* Prettier disables all conflicting rules, listing as last override so it takes precedence
* people kept ignoring that this was last so it's now defined outside of the overrides list
*/
/** eslint-disable-next-line */
module.exports.overrides.push({ files: ['**/*'], rules: require('eslint-config-prettier').rules });
/** PLEASE DON'T PUT THINGS AFTER THIS */
13 changes: 9 additions & 4 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,6 @@ packages/kbn-babel-plugin-package-imports @elastic/kibana-operations
packages/kbn-babel-preset @elastic/kibana-operations
packages/kbn-babel-register @elastic/kibana-operations
packages/kbn-babel-transform @elastic/kibana-operations
packages/kbn-bazel-packages @elastic/kibana-operations
packages/kbn-bazel-runner @elastic/kibana-operations
packages/kbn-cases-components @elastic/response-ops
packages/kbn-chart-icons @elastic/kibana-visualizations
Expand Down Expand Up @@ -942,13 +941,17 @@ packages/kbn-hapi-mocks @elastic/kibana-core
packages/kbn-health-gateway-server @elastic/kibana-core
packages/kbn-i18n @elastic/kibana-core
packages/kbn-i18n-react @elastic/kibana-core
packages/kbn-import-locator @elastic/kibana-operations
packages/kbn-import-resolver @elastic/kibana-operations
packages/kbn-interpreter @elastic/kibana-visualizations
packages/kbn-io-ts-utils @elastic/apm-ui
packages/kbn-jest-serializers @elastic/kibana-operations
packages/kbn-journeys @elastic/kibana-operations
packages/kbn-json-ast @elastic/kibana-operations
packages/kbn-kibana-manifest-schema @elastic/kibana-operations
packages/kbn-language-documentation-popover @elastic/kibana-visualizations
packages/kbn-lint-packages-cli @elastic/kibana-operations
packages/kbn-lint-ts-projects-cli @elastic/kibana-operations
packages/kbn-logging @elastic/kibana-core
packages/kbn-logging-mocks @elastic/kibana-core
packages/kbn-managed-vscode-config @elastic/kibana-operations
Expand All @@ -958,15 +961,18 @@ packages/kbn-monaco @elastic/kibana-global-experience
packages/kbn-optimizer @elastic/kibana-operations
packages/kbn-optimizer-webpack-helpers @elastic/kibana-operations
packages/kbn-osquery-io-ts-types @elastic/security-asset-management
packages/kbn-package-map @elastic/kibana-operations
packages/kbn-peggy @elastic/kibana-operations
packages/kbn-peggy-loader @elastic/kibana-operations
packages/kbn-performance-testing-dataset-extractor @elastic/kibana-performance-testing
packages/kbn-picomatcher @elastic/kibana-operations
packages/kbn-plugin-discovery @elastic/kibana-operations
packages/kbn-plugin-generator @elastic/kibana-operations
packages/kbn-plugin-helpers @elastic/kibana-operations
packages/kbn-react-field @elastic/kibana-app-services
packages/kbn-repo-file-maps @elastic/kibana-operations
packages/kbn-repo-info @elastic/kibana-operations
packages/kbn-repo-linter @elastic/kibana-operations
packages/kbn-repo-packages @elastic/kibana-operations
packages/kbn-repo-path @elastic/kibana-operations
packages/kbn-repo-source-classifier @elastic/kibana-operations
packages/kbn-repo-source-classifier-cli @elastic/kibana-operations
Expand All @@ -990,6 +996,7 @@ packages/kbn-securitysolution-t-grid @elastic/security-solution-platform
packages/kbn-securitysolution-utils @elastic/security-solution-platform
packages/kbn-server-http-tools @elastic/kibana-core
packages/kbn-server-route-repository @elastic/apm-ui
packages/kbn-set-map @elastic/kibana-operations
packages/kbn-shared-svg @elastic/apm-ui
packages/kbn-shared-ux-utility @elastic/kibana-global-experience
packages/kbn-slo-schema @elastic/actionable-observability
Expand All @@ -1006,8 +1013,6 @@ packages/kbn-test-subj-selector @elastic/kibana-operations
packages/kbn-timelion-grammar @elastic/kibana-visualizations
packages/kbn-tinymath @elastic/kibana-visualizations
packages/kbn-tooling-log @elastic/kibana-operations
packages/kbn-ts-project-linter @elastic/kibana-operations
packages/kbn-ts-project-linter-cli @elastic/kibana-operations
packages/kbn-ts-projects @elastic/kibana-operations
packages/kbn-ts-type-check-cli @elastic/kibana-operations
packages/kbn-typed-react-router-config @elastic/apm-ui
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ elastic-agent-*
fleet-server-*
elastic-agent.yml
fleet-server.yml
/packages/kbn-package-map/package-map.json
/packages/*/package-map.json
/packages/*/config-paths.json
/packages/kbn-synthetic-package-map/
**/.synthetics/
**/.journeys/
61 changes: 32 additions & 29 deletions kbn_pm/src/commands/bootstrap/bootstrap_command.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import { haveNodeModulesBeenManuallyDeleted, removeYarnIntegrityFileIfExists } f
import { setupRemoteCache } from './setup_remote_cache.mjs';
import { sortPackageJson } from './sort_package_json.mjs';
import { regeneratePackageMap } from './regenerate_package_map.mjs';
import { regenerateTsconfigPaths } from './regenerate_tsconfig_paths.mjs';
import { regenerateBaseTsconfig } from './regenerate_base_tsconfig.mjs';
import { packageDiscovery, pluginDiscovery } from './discovery.mjs';
import { validatePackageJson } from './validate_package_json.mjs';
import { discovery } from './discovery.mjs';
import { updatePackageJson } from './update_package_json.mjs';

/** @type {import('../../lib/command').Command} */
export const command = {
Expand Down Expand Up @@ -61,13 +62,33 @@ export const command = {
const forceInstall =
args.getBooleanValue('force-install') ?? (await haveNodeModulesBeenManuallyDeleted());

await Bazel.tryRemovingBazeliskFromYarnGlobal(log);
const [{ packages, plugins, tsConfigsPaths }] = await Promise.all([
// discover the location of packages, plugins, etc
await time('discovery', discovery),

// Install bazel machinery tools if needed
await Bazel.ensureInstalled(log);
(async () => {
await Bazel.tryRemovingBazeliskFromYarnGlobal(log);

// Setup remote cache settings in .bazelrc.cache if needed
await setupRemoteCache(log);
// Install bazel machinery tools if needed
await Bazel.ensureInstalled(log);

// Setup remote cache settings in .bazelrc.cache if needed
await setupRemoteCache(log);
})(),
]);

// generate the package map and package.json file, if necessary
await Promise.all([
time('regenerate package map', async () => {
await regeneratePackageMap(packages, plugins, log);
}),
time('regenerate tsconfig map', async () => {
await regenerateTsconfigPaths(tsConfigsPaths, log);
}),
time('update package json', async () => {
await updatePackageJson(packages, log);
}),
]);

// Bootstrap process for Bazel packages
// Bazel is now managing dependencies so yarn install
Expand All @@ -85,34 +106,16 @@ export const command = {
});
}

// discover the location of packages and plugins
const [plugins, packages] = await Promise.all([
time('plugin discovery', pluginDiscovery),
time('package discovery', packageDiscovery),
]);

// generate the package map which powers the resolver and several other features
// needed as an input to the bazel builds
await time('regenerate package map', async () => {
await regeneratePackageMap(packages, plugins, log);
});

await time('pre-build webpack bundles for packages', async () => {
await Bazel.buildWebpackBundles(log, { offline, quiet });
});

await time('regenerate tsconfig.base.json', async () => {
await regenerateBaseTsconfig();
});

await Promise.all([
time('sort package json', async () => {
await sortPackageJson();
time('regenerate tsconfig.base.json', async () => {
await regenerateBaseTsconfig();
}),
time('validate package json', async () => {
// now that deps are installed we can import `@kbn/yarn-lock-validator`
const { kibanaPackageJson } = External['@kbn/repo-info']();
await validatePackageJson(kibanaPackageJson, log);
time('sort package json', async () => {
await sortPackageJson(log);
}),
validate
? time('validate dependencies', async () => {
Expand Down
89 changes: 70 additions & 19 deletions kbn_pm/src/commands/bootstrap/discovery.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,84 @@
* Side Public License, v 1.
*/

import { REPO_ROOT } from '../../lib/paths.mjs';
import Path from 'path';
import Fs from 'fs';
import ChildProcess from 'child_process';
import { promisify } from 'util';

// we need to run these in order to generate the pkg map which is used by things
// like `@kbn/babel-register`, so we have to import the JS files directory and can't
// rely on `@kbn/babel-register`.
import { REPO_ROOT } from '../../lib/paths.mjs';
const execAsync = promisify(ChildProcess.execFile);

export async function packageDiscovery() {
const { discoverBazelPackages } = await import(
export async function discovery() {
const { getPluginSearchPaths, simpleKibanaPlatformPluginDiscovery } = await import(
// eslint-disable-next-line @kbn/imports/uniform_imports
'../../../../packages/kbn-bazel-packages/index.js'
'../../../../packages/kbn-plugin-discovery/index.js'
);

return await discoverBazelPackages(REPO_ROOT);
}

export async function pluginDiscovery() {
const { getPluginSearchPaths, simpleKibanaPlatformPluginDiscovery } = await import(
const { Package } = await import(
// we need to run this before we install node modules, so it can't rely on @kbn/* imports
// eslint-disable-next-line @kbn/imports/uniform_imports
'../../../../packages/kbn-plugin-discovery/index.js'
'../../../../packages/kbn-repo-packages/index.js'
);

const searchPaths = getPluginSearchPaths({
rootDir: REPO_ROOT,
examples: true,
oss: false,
testPlugins: true,
const proc = await execAsync('git', ['ls-files', '-comt', '--exclude-standard'], {
cwd: REPO_ROOT,
encoding: 'utf8',
maxBuffer: Infinity,
});

return simpleKibanaPlatformPluginDiscovery(searchPaths, []);
const paths = new Map();
/** @type {Map<string, Set<string>>} */
const filesByName = new Map();

for (const raw of proc.stdout.split('\n')) {
const line = raw.trim();
if (!line) {
continue;
}

const repoRel = line.slice(2); // trim the single char status and separating space from the line
const name = repoRel.split('/').pop();
if (name !== 'kibana.jsonc' && name !== 'tsconfig.json') {
continue;
}

const existingPath = paths.get(repoRel);
const path = existingPath ?? Path.resolve(REPO_ROOT, repoRel);
if (!existingPath) {
paths.set(repoRel, path);
}

let files = filesByName.get(name);
if (!files) {
files = new Set();
filesByName.set(name, files);
}

if (line.startsWith('C ')) {
// this line indicates that the previous path is changed in the working
// tree, so we need to determine if it was deleted and remove it if so
if (!Fs.existsSync(path)) {
files.delete(path);
}
} else {
files.add(path);
}
}

return {
plugins: simpleKibanaPlatformPluginDiscovery(
getPluginSearchPaths({
rootDir: REPO_ROOT,
examples: true,
oss: false,
testPlugins: true,
}),
[]
),
tsConfigsPaths: Array.from(filesByName.get('tsconfig.json') ?? new Set()),
packages: Array.from(filesByName.get('kibana.jsonc') ?? new Set())
.map((path) => Package.fromManifest(REPO_ROOT, path))
.sort((a, b) => a.id.localeCompare(b.id)),
};
}
2 changes: 1 addition & 1 deletion kbn_pm/src/commands/bootstrap/regenerate_base_tsconfig.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { REPO_ROOT } from '../../lib/paths.mjs';
import External from '../../lib/external_packages.js';

export async function regenerateBaseTsconfig() {
const pkgMap = External['@kbn/package-map']().readPackageMap();
const pkgMap = External['@kbn/repo-packages']().readPackageMap();
const tsconfigPath = Path.resolve(REPO_ROOT, 'tsconfig.base.json');
const lines = (await Fsp.readFile(tsconfigPath, 'utf-8')).split('\n');

Expand Down
Loading

0 comments on commit d6be4a4

Please sign in to comment.