Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2025: Add check to ensure files end in .json #2026

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
22 changes: 22 additions & 0 deletions src/checks/generic/json-file-extension.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { basename, extname } from 'path';
import { exit } from 'process';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a stray import.

import { Check } from '../../types/checks';

const check: Check = {
id: 'json-file-extension',
desc: 'Data files should end with `.json` (#2025)',
url: 'https://github.com/datenanfragen/data/#data-formats',
severity: 'ERROR',
run: (json, ctx) => {
const filename = basename(ctx.file_path);
const file_extension = extname(ctx.file_path)

if (file_extension != ".json")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (file_extension != ".json")
if (file_extension !== ".json")

return {
message: `File \`${filename}\` should end in .json. Please rename the file.`,
location: { start: { line: 1 } },
Comment on lines +16 to +17
Copy link
Member

@mal-tee mal-tee Dec 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't make much sense as a suggestion from the reviewbot, but afaik there is no way for a code suggestion to rename a file in GH. @baltpeter do you have an opinion on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik there is no way for a code suggestion to rename a file in GH.

No, there isn't. I agree, this should just be a comment without a suggestion.

suggestions: [basename(filename, file_extension) + '.json'],
};
},
};
export default check;
5 changes: 5 additions & 0 deletions src/checks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ const linters = async () => {
url: 'https://github.com/datenanfragen/data/blob/master/src/checks/pack',
path_filter: (path: string) => path.startsWith('company-packs/'),
},
generic: {
davenewham marked this conversation as resolved.
Show resolved Hide resolved
checks: await importChecks('generic'),
url: 'https://github.com/datenanfragen/data/blob/master/src/checks/generic',
path_filter: (path: string) => path.startsWith('companies/') || path.startsWith('company-packs/') || path.startsWith('supervisory-authorities/'),
davenewham marked this conversation as resolved.
Show resolved Hide resolved
},
};
};
export default linters;
4 changes: 2 additions & 2 deletions src/checks/record/match-filename-slug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ const check: Check = {
url: 'https://github.com/datenanfragen/data/#data-formats',
severity: 'ERROR',
run: (json, ctx) => {
const filename = basename(ctx.file_path);
if (json.slug + '.json' !== filename)
const filename = basename(ctx.file_path, '.json');
if (json.slug !== filename)
return {
message: `Filename \`${filename}\` does not match slug \`${json.slug}\`.

Expand Down
2 changes: 1 addition & 1 deletion src/test-records.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const validate = async (dir: string) => {
const check_results: RdjsonLine[] = [];

const linters = await _linters();
const files = glob.sync(`${dir}/*.json`);
const files = glob.sync(`${dir}/*`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worried this might create a problem on systems that like to automatically place files in directories (Thumbs.db, .DS_Store etc.).

We should ignore everything in the .gitignore in this matcher. Unfortunately, our glob library doesn't support that as far as I can see. It might be worth switching to something like https://github.com/sindresorhus/globby.


for (const f of files) {
const file_content = fs.readFileSync(f).toString();
Expand Down