Linter: Custom eslint rule to prevent in-repo plugins from reaching into neighbouring plugin code (#112248)
* feat(linter-rule): prevent cross plugin relative path imports * docs(readme): document new grafana/no-plugin-external-import-paths rule * chore(test): add an NPM script to run linter rule tests * test(linter-rule): prevent cross plugin relative path imports * docs(eslint-config): add a commented out example of new rule usage
This commit is contained in:
@@ -599,4 +599,17 @@ module.exports = [
|
||||
],
|
||||
},
|
||||
},
|
||||
|
||||
// {
|
||||
// name: 'grafana/plugin-external-import-paths',
|
||||
// files: [
|
||||
// 'public/app/plugins/panel/histogram/**/*.{ts,tsx}',
|
||||
// ],
|
||||
// plugins: {
|
||||
// '@grafana': grafanaPlugin,
|
||||
// },
|
||||
// rules: {
|
||||
// '@grafana/no-plugin-external-import-paths': 'error',
|
||||
// },
|
||||
// },
|
||||
];
|
||||
|
||||
@@ -140,3 +140,44 @@ export default storyConfig;
|
||||
const storyConfig = { title: 'Components/Forms/Button' };
|
||||
export default storyConfig;
|
||||
```
|
||||
|
||||
### `no-plugin-external-import-paths`
|
||||
|
||||
Prevent plugins from importing anything outside their own directory.
|
||||
|
||||
This rule enforces strict plugin isolation by preventing plugins from importing anything that reaches outside their own plugin directory. This helps maintain clean plugin boundaries and prevents tight coupling between plugins and other parts of the codebase.
|
||||
|
||||
The rule automatically detects the current plugin directory from the file path and blocks any relative imports that would reach outside that directory.
|
||||
|
||||
The rule is applied to specific plugins by configuring the `files` pattern in the ESLint configuration, similar to `grafana/decoupled-plugins-overrides`.
|
||||
|
||||
#### Examples
|
||||
|
||||
```tsx
|
||||
// Bad ❌ - Importing from sibling plugin
|
||||
import { getDataLinks } from '../status-history/utils';
|
||||
import { isTooltipScrollable } from '../timeseries/utils';
|
||||
|
||||
// Bad ❌ - Importing from Grafana core
|
||||
import { something } from '../../../features/dashboard/state';
|
||||
|
||||
// Bad ❌ - Importing from outside plugin directory
|
||||
import { other } from '../some-other-folder/utils';
|
||||
|
||||
// Good ✅ - Importing from same plugin
|
||||
import { someUtil } from './utils';
|
||||
import { Component } from './Component';
|
||||
import { helper } from './subfolder/helper';
|
||||
|
||||
// Good ✅ - Importing from external packages
|
||||
import React from 'react';
|
||||
import { Button } from '@grafana/ui';
|
||||
```
|
||||
|
||||
#### Error Message
|
||||
|
||||
When a violation is detected, the rule reports:
|
||||
|
||||
```
|
||||
Import '../status-history/utils' reaches outside the 'histogram' plugin directory. Plugins should only import from external dependencies or relative paths within their own directory.
|
||||
```
|
||||
|
||||
@@ -4,6 +4,7 @@ const noUnreducedMotion = require('./rules/no-unreduced-motion.cjs');
|
||||
const themeTokenUsage = require('./rules/theme-token-usage.cjs');
|
||||
const noRestrictedImgSrcs = require('./rules/no-restricted-img-srcs.cjs');
|
||||
const consistentStoryTitles = require('./rules/consistent-story-titles.cjs');
|
||||
const noPluginExternalImportPaths = require('./rules/no-plugin-external-import-paths.cjs');
|
||||
|
||||
module.exports = {
|
||||
rules: {
|
||||
@@ -13,5 +14,6 @@ module.exports = {
|
||||
'theme-token-usage': themeTokenUsage,
|
||||
'no-restricted-img-srcs': noRestrictedImgSrcs,
|
||||
'consistent-story-titles': consistentStoryTitles,
|
||||
'no-plugin-external-import-paths': noPluginExternalImportPaths,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
export default {
|
||||
testEnvironment: 'node',
|
||||
testMatch: ['<rootDir>/**/*.test.js'],
|
||||
};
|
||||
@@ -2,11 +2,13 @@
|
||||
"name": "@grafana/eslint-plugin",
|
||||
"description": "ESLint rules for use within the Grafana repo. Not suitable (or supported) for external use.",
|
||||
"version": "12.4.0-pre",
|
||||
"type": "module",
|
||||
"main": "./index.cjs",
|
||||
"author": "Grafana Labs",
|
||||
"license": "Apache-2.0",
|
||||
"scripts": {
|
||||
"typecheck": "tsc --emitDeclarationOnly false --noEmit"
|
||||
"typecheck": "tsc --emitDeclarationOnly false --noEmit",
|
||||
"test": "NODE_OPTIONS='--experimental-vm-modules' jest"
|
||||
},
|
||||
"repository": {
|
||||
"type": "git",
|
||||
@@ -19,6 +21,7 @@
|
||||
"devDependencies": {
|
||||
"@typescript-eslint/types": "^8.9.0",
|
||||
"eslint": "9.32.0",
|
||||
"jest": "29.7.0",
|
||||
"tslib": "2.8.1"
|
||||
},
|
||||
"private": true
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
// @ts-check
|
||||
/** @typedef {import('@typescript-eslint/utils').TSESTree.ImportDeclaration} ImportDeclaration */
|
||||
const { ESLintUtils } = require('@typescript-eslint/utils');
|
||||
const path = require('path');
|
||||
|
||||
const createRule = ESLintUtils.RuleCreator(
|
||||
(name) => `https://github.com/grafana/grafana/blob/main/packages/grafana-eslint-rules/README.md#${name}`
|
||||
);
|
||||
|
||||
/**
|
||||
* Extract the plugin root directory from the file path
|
||||
* @param {string} filePath - The file path being linted
|
||||
* @returns {string|null} - The plugin root directory or null if not in a plugin directory
|
||||
*/
|
||||
function getPluginRootDirectory(filePath) {
|
||||
const pluginMatch = filePath.match(/\/plugins\/(?:panel|datasource)\/([^/]+)\//);
|
||||
if (pluginMatch) {
|
||||
const pluginName = pluginMatch[1];
|
||||
const pluginType = pluginMatch[0].includes('/panel/') ? 'panel' : 'datasource';
|
||||
|
||||
const pluginDirPath = `/plugins/${pluginType}/${pluginName}`;
|
||||
const pluginDirStart = filePath.indexOf(pluginDirPath);
|
||||
if (pluginDirStart !== -1) {
|
||||
const pluginRoot = filePath.substring(0, pluginDirStart + pluginDirPath.length);
|
||||
return path.isAbsolute(pluginRoot) ? pluginRoot : path.resolve(pluginRoot);
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if an import path reaches outside the plugin's root directory boundaries
|
||||
* @param {string} importPath - The import path to check
|
||||
* @param {string} currentFilePath - The current file path being linted
|
||||
* @param {string} pluginRoot - The plugin root directory
|
||||
* @returns {boolean} - True if the import goes outside plugin boundaries
|
||||
*/
|
||||
function isImportOutsidePluginBoundaries(importPath, currentFilePath, pluginRoot) {
|
||||
const isRelativeImport = importPath.startsWith('./') || importPath.startsWith('../');
|
||||
if (!isRelativeImport) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const currentDir = path.dirname(currentFilePath);
|
||||
const resolvedPath = path.resolve(currentDir, importPath);
|
||||
|
||||
const normalizedResolvedPath = path.normalize(resolvedPath);
|
||||
const normalizedPluginRoot = path.normalize(pluginRoot);
|
||||
|
||||
return !normalizedResolvedPath.startsWith(normalizedPluginRoot);
|
||||
}
|
||||
|
||||
const noRestrictedPeerPluginPathsRule = createRule({
|
||||
create(context) {
|
||||
const currentFilePath = context.getFilename();
|
||||
const pluginRoot = getPluginRootDirectory(currentFilePath);
|
||||
|
||||
if (!pluginRoot) {
|
||||
return {};
|
||||
}
|
||||
|
||||
return {
|
||||
/** @param {ImportDeclaration} node */
|
||||
ImportDeclaration(node) {
|
||||
const importPath = node.source.value;
|
||||
|
||||
if (
|
||||
typeof importPath === 'string' &&
|
||||
isImportOutsidePluginBoundaries(importPath, currentFilePath, pluginRoot)
|
||||
) {
|
||||
return context.report({
|
||||
node: node.source,
|
||||
messageId: 'importOutsidePluginBoundaries',
|
||||
data: {
|
||||
importPath,
|
||||
pluginRoot: path.basename(pluginRoot),
|
||||
},
|
||||
});
|
||||
}
|
||||
},
|
||||
};
|
||||
},
|
||||
name: 'no-plugin-external-import-paths',
|
||||
meta: {
|
||||
type: 'problem',
|
||||
docs: {
|
||||
description: 'Disallow imports that reach outside plugin root directory boundaries',
|
||||
},
|
||||
messages: {
|
||||
importOutsidePluginBoundaries:
|
||||
"Import '{{importPath}}' reaches outside the '{{pluginRoot}}' plugin directory. Plugins should only import from external dependencies or relative paths within their own directory.",
|
||||
},
|
||||
schema: [],
|
||||
},
|
||||
defaultOptions: [],
|
||||
});
|
||||
|
||||
module.exports = noRestrictedPeerPluginPathsRule;
|
||||
@@ -0,0 +1,81 @@
|
||||
import { RuleTester } from 'eslint';
|
||||
|
||||
import rule from '../rules/no-plugin-external-import-paths.cjs';
|
||||
|
||||
RuleTester.setDefaultConfig({
|
||||
languageOptions: {
|
||||
ecmaVersion: 2020,
|
||||
sourceType: 'module',
|
||||
},
|
||||
});
|
||||
|
||||
const ruleTester = new RuleTester();
|
||||
|
||||
ruleTester.run('eslint no-plugin-external-import-paths', rule, {
|
||||
valid: [
|
||||
{
|
||||
name: 'external npm package import',
|
||||
filename: 'public/app/plugins/panel/histogram/HistogramTooltip.tsx',
|
||||
code: "import React from 'react';",
|
||||
},
|
||||
{
|
||||
name: 'grafana package import',
|
||||
filename: 'public/app/plugins/panel/histogram/HistogramTooltip.tsx',
|
||||
code: "import { Button } from '@grafana/ui';",
|
||||
},
|
||||
{
|
||||
name: 'same plugin file import',
|
||||
filename: 'public/app/plugins/panel/histogram/HistogramTooltip.tsx',
|
||||
code: "import { someUtil } from './utils';",
|
||||
},
|
||||
{
|
||||
name: 'same plugin subdirectory import',
|
||||
filename: 'public/app/plugins/panel/histogram/components/HistogramTooltip.tsx',
|
||||
code: "import { Component } from '../Component';",
|
||||
},
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
name: 'sibling plugin import',
|
||||
filename: 'public/app/plugins/panel/histogram/HistogramTooltip.tsx',
|
||||
code: "import { getDataLinks } from '../status-history/utils';",
|
||||
errors: [
|
||||
{
|
||||
messageId: 'importOutsidePluginBoundaries',
|
||||
data: {
|
||||
importPath: '../status-history/utils',
|
||||
pluginRoot: 'histogram',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'grafana core import',
|
||||
filename: 'public/app/plugins/panel/histogram/HistogramTooltip.tsx',
|
||||
code: "import { something } from '../../../features/dashboard/state';",
|
||||
errors: [
|
||||
{
|
||||
messageId: 'importOutsidePluginBoundaries',
|
||||
data: {
|
||||
importPath: '../../../features/dashboard/state',
|
||||
pluginRoot: 'histogram',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'datasource plugin sibling import',
|
||||
filename: 'public/app/plugins/datasource/loki/datasource.ts',
|
||||
code: "import { something } from '../prometheus/utils';",
|
||||
errors: [
|
||||
{
|
||||
messageId: 'importOutsidePluginBoundaries',
|
||||
data: {
|
||||
importPath: '../prometheus/utils',
|
||||
pluginRoot: 'loki',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
Reference in New Issue
Block a user