[6.1] [precommit/casingCheck] enhance casing check for less false-positives (#15102) (#15438)

* [precommit/casingCheck] enhance casing check for less false-positives

* [dev/File] add tests
This commit is contained in:
Spencer 2017-12-05 17:36:27 -07:00 committed by GitHub
parent a1a7379937
commit c3d217e8f5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 346 additions and 22 deletions

73
src/dev/__tests__/file.js Normal file
View file

@ -0,0 +1,73 @@
import { resolve } from 'path';
import expect from 'expect.js';
import { File } from '../file';
const HERE = resolve(__dirname, __filename);
describe('dev/File', () => {
describe('constructor', () => {
it('throws if path is not a string', () => {
expect(() => new File()).to.throwError();
expect(() => new File(1)).to.throwError();
expect(() => new File(false)).to.throwError();
expect(() => new File(null)).to.throwError();
});
});
describe('#getRelativePath()', () => {
it('returns the path relative to the repo root', () => {
const file = new File(HERE);
expect(file.getRelativePath()).to.eql('src/dev/__tests__/file.js');
});
});
describe('#isJs()', () => {
it('returns true if extension is .js', () => {
const file = new File('file.js');
expect(file.isJs()).to.eql(true);
});
it('returns false if extension is .xml', () => {
const file = new File('file.xml');
expect(file.isJs()).to.eql(false);
});
it('returns false if extension is .css', () => {
const file = new File('file.css');
expect(file.isJs()).to.eql(false);
});
it('returns false if extension is .html', () => {
const file = new File('file.html');
expect(file.isJs()).to.eql(false);
});
it('returns false if file has no extension', () => {
const file = new File('file');
expect(file.isJs()).to.eql(false);
});
});
describe('#getRelativeParentDirs()', () => {
it('returns the parents of a file, stopping at the repo root, in descending order', () => {
const file = new File(HERE);
expect(file.getRelativeParentDirs()).to.eql([
'src/dev/__tests__',
'src/dev',
'src',
]);
});
});
describe('#toString()', () => {
it('returns the relativePath', () => {
const file = new File(HERE);
expect(file.toString()).to.eql(file.getRelativePath());
});
});
describe('#toJSON()', () => {
it('returns the relativePath', () => {
const file = new File(HERE);
expect(file.toJSON()).to.eql(file.getRelativePath());
});
});
});

View file

@ -1,5 +1,6 @@
import { CLIEngine } from 'eslint';
import { matchesAnyGlob } from '../globs';
import { DEFAULT_ESLINT_PATHS } from './default_eslint_paths';
/**
@ -19,8 +20,9 @@ export function pickFilesToLint(log, files) {
const sourcePathGlobs = cli.resolveFileGlobPatterns(DEFAULT_ESLINT_PATHS);
return files.filter(file => {
const isNormallyLinted = file.matchesAnyGlob(sourcePathGlobs);
const isExplicitlyIgnored = isNormallyLinted && cli.isPathIgnored(file.getRelativePath());
const path = file.getRelativePath();
const isNormallyLinted = matchesAnyGlob(path, sourcePathGlobs);
const isExplicitlyIgnored = isNormallyLinted && cli.isPathIgnored(path);
if (isNormallyLinted && !isExplicitlyIgnored) {
log.debug('linting %j', file);

View file

@ -1,6 +1,4 @@
import { resolve, relative, extname } from 'path';
import minimatch from 'minimatch';
import { dirname, join, resolve, relative, extname } from 'path';
import { REPO_ROOT } from './constants';
@ -19,14 +17,23 @@ export class File {
return this._ext === '.js';
}
matchesRegex(regex) {
return this._relativePath.match(regex);
}
getRelativeParentDirs() {
const parents = [];
matchesAnyGlob(globs) {
return globs.some(pattern => minimatch(this._relativePath, pattern, {
dot: true
}));
while (true) {
const parent = parents.length
// NOTE: resolve() produces absolute paths, so we have to use join()
? join(parents[parents.length - 1], '..')
: dirname(this._relativePath);
if (parent === '..' || parent === '.') {
break;
} else {
parents.push(parent);
}
}
return parents;
}
toString() {

View file

@ -0,0 +1,23 @@
import glob from 'glob';
import { run } from './run';
import { File } from './file';
import { REPO_ROOT } from './constants';
import { checkFileCasing } from './precommit_hook/check_file_casing';
run(async ({ log }) => {
const paths = glob.sync('**/*', {
cwd: REPO_ROOT,
nodir: true,
ignore: [
'**/node_modules/**/*',
'optimize/**/*',
'esvm/**/*',
'data/**/*',
]
});
const files = paths.map(path => new File(path));
await checkFileCasing(log, files);
});

7
src/dev/globs.js Normal file
View file

@ -0,0 +1,7 @@
import minimatch from 'minimatch';
export function matchesAnyGlob(path, globs) {
return globs.some(pattern => minimatch(path, pattern, {
dot: true
}));
}

View file

@ -0,0 +1,176 @@
/**
* These patterns are used to identifiy files that are not supposed
* to be snake_case because their names are determined by other
* systems or rules.
*
* @type {Array}
*/
export const IGNORE_FILE_GLOBS = [
'docs/**/*',
'bin/**/*',
'**/+([A-Z_]).md',
'**/*.txt',
'Gruntfile.js',
'tasks/config/**/*',
'tasks/build/docker/docs/{Dockerfile,docker-compose.yml}',
];
/**
* These patterns are matched against directories and indicate
* explicit folders that are NOT supposed to use snake_case.
*
* When a file in one of these directories is checked, the directory
* matched by these patterns is removed from the path before
* the casing check so that the files casing is still checked. This
* allows folders like `ui_framework/generator-kui` to exist, which
* is named to match the npm package and follow the kebab-casing
* convention there, but allows us to still verify that files within
* that directory use snake_case
*
* @type {Array}
*/
export const IGNORE_DIRECTORY_GLOBS = [
'**/webpackShims',
'src/babel-*',
'packages/eslint-*',
'ui_framework/generator-kui',
'src/ui/public/angular-bootstrap',
'src/ui/public/flot-charts',
'src/ui/public/utils/lodash-mixins',
'test/functional/fixtures/es_archiver/visualize_source-filters',
];
/**
* DO NOT ADD FILES TO THIS LIST!!
*
* Use the other configs if the file really shouldn't be snake_case.
*
* These paths identify filenames that would be flagged by the current
* rules but were in violation before we started properly enforcing these
* rules. They will not cause errors but will log warnings because they
* will hopefully be updated to use snake_case in the future.
*
* IDEALLY will will be able to trim this list over time
*
* @type {Array}
*/
export const TEMPORARILY_IGNORED_PATHS = [
'src/core_plugins/console/public/src/directives/helpExample.txt',
'src/core_plugins/console/public/src/sense_editor/theme-sense-dark.js',
'src/core_plugins/console/public/tests/webpackShims/qunit-1.10.0.css',
'src/core_plugins/console/public/tests/webpackShims/qunit-1.10.0.js',
'src/core_plugins/console/public/webpackShims/ace/ext-language_tools.js',
'src/core_plugins/console/public/webpackShims/ace/ext-searchbox.js',
'src/core_plugins/console/public/webpackShims/ace/mode-json.js',
'src/core_plugins/console/public/webpackShims/ace/mode-yaml.js',
'src/core_plugins/console/public/webpackShims/ace/worker-json.js',
'src/core_plugins/console/public/webpackShims/ui-bootstrap-custom.js',
'src/core_plugins/input_control_vis/public/images/icon-input-control.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-area.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-donut.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-gauge.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-goal.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-heatmap.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-horizontal.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-line.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-number.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-pie.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-tilemap.svg',
'src/core_plugins/kbn_vislib_vis_types/public/images/icon-vertical.svg',
'src/core_plugins/kibana/public/assets/play-circle.svg',
'src/core_plugins/markdown_vis/public/images/icon-markdown.svg',
'src/core_plugins/metric_vis/public/images/icon-number.svg',
'src/core_plugins/metrics/public/images/icon-visualbuilder.svg',
'src/core_plugins/region_map/public/images/icon-vector-map.svg',
'src/core_plugins/table_vis/public/images/icon-table.svg',
'src/core_plugins/tagcloud/public/images/icon-tagcloud.svg',
'src/core_plugins/tests_bundle/webpackShims/angular-mocks.js',
'src/core_plugins/tile_map/public/__tests__/scaledCircleMarkers.png',
'src/core_plugins/tile_map/public/__tests__/shadedCircleMarkers.png',
'src/core_plugins/tile_map/public/__tests__/shadedGeohashGrid.png',
'src/core_plugins/tile_map/public/images/icon-tilemap.svg',
'src/core_plugins/timelion/public/images/icon-timelion.svg',
'src/core_plugins/timelion/server/lib/asSorted.js',
'src/core_plugins/timelion/server/lib/unzipPairs.js',
'src/core_plugins/timelion/server/series_functions/__tests__/fixtures/bucketList.js',
'src/core_plugins/timelion/server/series_functions/__tests__/fixtures/seriesList.js',
'src/core_plugins/timelion/server/series_functions/__tests__/fixtures/tlConfig.js',
'src/fixtures/config_upgrade_from_4.0.0_to_4.0.1-snapshot.json',
'src/fixtures/vislib/mock_data/terms/_seriesMultiple.js',
'src/jest/babelTransform.js',
'src/ui/i18n/__tests__/fixtures/translations/test_plugin_1/es-ES.json',
'src/ui/public/angular-bootstrap/accordion/accordion-group.html',
'src/ui/public/angular-bootstrap/bindHtml/bindHtml.js',
'src/ui/public/angular-bootstrap/tooltip/tooltip-html-unsafe-popup.html',
'src/ui/public/angular-bootstrap/tooltip/tooltip-popup.html',
'src/ui/public/angular-bootstrap/typeahead/typeahead-match.html',
'src/ui/public/angular-bootstrap/typeahead/typeahead-popup.html',
'src/ui/public/assets/favicons/android-chrome-192x192.png',
'src/ui/public/assets/favicons/android-chrome-256x256.png',
'src/ui/public/assets/favicons/apple-touch-icon.png',
'src/ui/public/assets/favicons/favicon-16x16.png',
'src/ui/public/assets/favicons/favicon-32x32.png',
'src/ui/public/assets/favicons/mstile-150x150.png',
'src/ui/public/assets/favicons/safari-pinned-tab.svg',
'src/ui/public/directives/__tests__/confirm-click.js',
'src/ui/public/dragula/gu-dragula.less',
'src/ui/public/field_format_editor/editors/url/icons/flag-icon.LICENSE',
'src/ui/public/icons/beats-color.svg',
'src/ui/public/icons/beats-gray.svg',
'src/ui/public/icons/elasticsearch-color.svg',
'src/ui/public/icons/elasticsearch-gray.svg',
'src/ui/public/icons/kibana-color.svg',
'src/ui/public/icons/kibana-gray.svg',
'src/ui/public/icons/logstash-color.svg',
'src/ui/public/icons/logstash-gray.svg',
'src/ui/public/icons/security-gray.svg',
'src/ui/public/query_bar/lib/queryLanguages.js',
'src/ui/public/styles/bootstrap/component-animations.less',
'src/ui/public/styles/bootstrap/input-groups.less',
'src/ui/public/styles/bootstrap/list-group.less',
'src/ui/public/styles/bootstrap/mixins/background-variant.less',
'src/ui/public/styles/bootstrap/mixins/border-radius.less',
'src/ui/public/styles/bootstrap/mixins/center-block.less',
'src/ui/public/styles/bootstrap/mixins/grid-framework.less',
'src/ui/public/styles/bootstrap/mixins/hide-text.less',
'src/ui/public/styles/bootstrap/mixins/list-group.less',
'src/ui/public/styles/bootstrap/mixins/nav-divider.less',
'src/ui/public/styles/bootstrap/mixins/nav-vertical-align.less',
'src/ui/public/styles/bootstrap/mixins/progress-bar.less',
'src/ui/public/styles/bootstrap/mixins/reset-filter.less',
'src/ui/public/styles/bootstrap/mixins/reset-text.less',
'src/ui/public/styles/bootstrap/mixins/responsive-visibility.less',
'src/ui/public/styles/bootstrap/mixins/tab-focus.less',
'src/ui/public/styles/bootstrap/mixins/table-row.less',
'src/ui/public/styles/bootstrap/mixins/text-emphasis.less',
'src/ui/public/styles/bootstrap/mixins/text-overflow.less',
'src/ui/public/styles/bootstrap/mixins/vendor-prefixes.less',
'src/ui/public/styles/bootstrap/progress-bars.less',
'src/ui/public/styles/bootstrap/responsive-utilities.less',
'src/ui/public/styles/dark-theme.less',
'src/ui/public/styles/dark-variables.less',
'src/ui/public/styles/fonts/glyphicons-halflings-regular.eot',
'src/ui/public/styles/fonts/glyphicons-halflings-regular.svg',
'src/ui/public/styles/fonts/glyphicons-halflings-regular.ttf',
'src/ui/public/styles/fonts/glyphicons-halflings-regular.woff',
'src/ui/public/styles/fonts/glyphicons-halflings-regular.woff2',
'src/ui/public/styles/list-group-menu.less',
'src/ui/public/styles/react-input-range.less',
'src/ui/public/styles/react-select.less',
'src/ui/public/styles/theme/font-awesome.less',
'src/ui/public/styles/variables/bootstrap-mods.less',
'src/ui/public/styles/variables/for-theme.less',
'src/ui/public/typeahead/partials/typeahead-items.html',
'src/ui/public/utils/migrateLegacyQuery.js',
'test/functional/apps/management/exports/_import_objects-conflicts.json',
'ui_framework/doc_site/src/images/elastic-logo.svg',
'ui_framework/doc_site/src/images/hint-arrow.svg',
'ui_framework/doc_site/src/images/react-logo.svg',
'webpackShims/angular-ui-select.js',
'webpackShims/elasticsearch-browser.js',
'webpackShims/moment-timezone.js',
'webpackShims/ui-bootstrap.js',
];

View file

@ -1,12 +1,14 @@
import { relative } from 'path';
import { createFailError } from '../run';
import { matchesAnyGlob } from '../globs';
import {
IGNORE_DIRECTORY_GLOBS,
IGNORE_FILE_GLOBS,
TEMPORARILY_IGNORED_PATHS,
} from './casing_check_config';
const NON_SNAKE_CASE_RE = /[A-Z \-]/;
const ALLOW_NON_SNAKE_CASE = [
'docs/**/*',
'packages/eslint-*/**/*',
'src/babel-*/**/*',
'tasks/config/**/*',
];
function listFileNames(files) {
return files
@ -14,6 +16,30 @@ function listFileNames(files) {
.join('\n');
}
/**
* IGNORE_DIRECTORY_GLOBS patterns match directories which should
* be ignored from casing validation. When one of the parent directories
* of a file matches these rules this function strips it from the
* path that is validated.
*
* if `file = new File('foo/bar/BAZ/index.js')` and `/foo/bar/BAZ`
* is matched by an `IGNORE_DIRECTORY_GLOBS` pattern then this
* function will return 'index.js' and only that part of the path
* will be validated.
*
* @param {File} file
* @return {string} pathToCheck
*/
function getPathWithoutIgnoredParents(file) {
for (const parent of file.getRelativeParentDirs()) {
if (matchesAnyGlob(parent, IGNORE_DIRECTORY_GLOBS)) {
return relative(parent, file.getRelativePath());
}
}
return file.getRelativePath();
}
/**
* Check that all passed File objects are using valid casing. Every
* file SHOULD be using snake_case but some files are allowed to stray:
@ -33,12 +59,22 @@ export async function checkFileCasing(log, files) {
const warnings = [];
files.forEach(file => {
const invalid = file.matchesRegex(NON_SNAKE_CASE_RE);
const allowed = file.matchesAnyGlob(ALLOW_NON_SNAKE_CASE);
const path = file.getRelativePath();
if (TEMPORARILY_IGNORED_PATHS.includes(path)) {
warnings.push(file);
return;
}
const ignored = matchesAnyGlob(path, IGNORE_FILE_GLOBS);
if (ignored) {
log.debug('%j ignored', file);
return;
}
const invalid = NON_SNAKE_CASE_RE.test(getPathWithoutIgnoredParents(file));
if (!invalid) {
log.debug('%j uses valid casing', file);
} else if (allowed) {
warnings.push(file);
} else {
errors.push(file);
}