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

Convert to TypeScript #339

Merged
merged 13 commits into from
Feb 11, 2024
90 changes: 90 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
"name": "Kristian Dupont",
"url": "http://www.kristiandupont.com"
},
"main": "build/engine.js",
"types": "build/engine.d.ts",
"main": "build/index.js",
"types": "build/index.d.ts",
Comment on lines +19 to +20
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

Changed "main" and "types" here to export the Rule and the Config types from the package root. The processDatabase function is exported as before.

"bin": {
"schemalint": "./bin/schemalint"
},
Expand Down Expand Up @@ -45,6 +45,7 @@
},
"devDependencies": {
"@kristiandupont/dev-deps": "^2.20.0",
"@types/pg": "^8.11.0",
"@types/ramda": "0.29.10",
"np": "9.2.0",
"testcontainers": "10.7.1"
Expand Down
76 changes: 76 additions & 0 deletions src/Config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { ClientConfig } from "pg";

/**
* The configuration for schemalint.
*/
type Config = {
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

I chose the short name Config. Would you prefer a more descriptive name, for example SchemalintConfig?

Copy link
Owner

Choose a reason for hiding this comment

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

I think this is fine. It will likely only be used in configuration files where it will be clear what's being referenced.

/**
* The connection configuration for the database.
* @see https://node-postgres.com/apis/client
*/
connection: ClientConfig;
/**
* The plugins to be used.
*/
plugins?: string[];
/**
* The rules to be used.
*/
rules: Record<string, RuleConfig>;
/**
* The schemas to be linted.
*/
schemas: SchemaConfig[];
/**
* The configuration for ignoring problems.
*/
ignores?: IgnoreConfig[];
};

/**
* A schema to be linted.
*/
export type SchemaConfig = {
/**
* The name of the schema to be linted.
*/
name: string;
/**
* The rules to be used spefically for this schema. These rules will be merged with the global rules.
*/
rules?: Record<string, RuleConfig>;
};

/**
* A rule configuration. The first element is the severity of the rule, and the rest of the elements are the options for the rule.
*/
export type RuleConfig = [Severity, ...unknown[]];

/**
* The severity of a rule. `off` means the rule is disabled, `error` means the rule is enabled.
*/
export type Severity = "off" | "error";
Comment on lines +49 to +52
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

In fact, any value other than "error" can be specified to disable the rule, but I chose the value "off" to match ESLint. Does it matter?


/**
* A configuration for ignoring problems.
*/
export type IgnoreConfig = {
/**
* The rule name to ignore. `rule` or `rulePattern` must be provided.
*/
rule?: string;
/**
* A pattern to match against the rule name. `rule` or `rulePattern` must be provided.
*/
rulePattern?: string;
/**
* The identifier to ignore. `identifier` or `identifierPattern` must be provided.
*/
identifier?: string;
/**
* A pattern to match against the identifier. `identifier` or `identifierPattern` must be provided.
*/
identifierPattern?: string;
};

export default Config;
6 changes: 4 additions & 2 deletions src/Rule.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { Schema } from "extract-pg-schema";

export type Reporter = (p: {
export type Issue = {
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

I named this object Issue. Doesn't that sound strange? Another idea is Problem.

rule: string;
identifier: string;
message: string;
suggestedMigration?: string;
}) => void;
};

export type Reporter = (p: Issue) => void;

type Rule = {
name: string;
Expand Down
8 changes: 4 additions & 4 deletions src/cli.js → src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import chalk from "chalk";
import optionator from "optionator";
import path from "path";

import { version } from "../package.json";
import { processDatabase } from "./engine";
// @ts-ignore
const { version } = require("../package.json");

async function main() {
const o = optionator({
Expand Down Expand Up @@ -34,11 +33,11 @@ async function main() {
],
});

let options;
let options: any;

try {
options = o.parseArgv(process.argv);
} catch (error) {
} catch (error: any) {
console.error(error.message);
process.exit(1);
}
Expand All @@ -60,6 +59,7 @@ async function main() {
);

try {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const config = require(configFile);
const exitCode = await processDatabase(config);
process.exit(exitCode);
Expand Down
78 changes: 44 additions & 34 deletions src/engine.js → src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,23 @@ import { extractSchemas } from "extract-pg-schema";
import path from "path";
import { indexBy, keys, prop, values } from "ramda";

import Config from "./Config";
import Rule, { Issue, Reporter } from "./Rule";
import * as builtinRules from "./rules";

function consoleReporter({ rule, identifier, message }) {
type IgnoreMatcher = (rule: string, identifier: string) => boolean;

function consoleReporter({ rule, identifier, message }: Issue) {
console.error(
`${chalk.yellow(identifier)}: error ${chalk.red(rule)} : ${message}`,
);
}

let anyIssues = false;
const suggestedMigrations = [];
const suggestedMigrations: string[] = [];

const createReportFunction =
(reporter, ignoreMatchers) =>
(reporter: Reporter, ignoreMatchers: IgnoreMatcher[]): Reporter =>
({ rule, identifier, message, suggestedMigration }) => {
if (ignoreMatchers.some((im) => im(rule, identifier))) {
// This one is ignored.
Expand All @@ -36,8 +40,11 @@ export async function processDatabase({
rules,
schemas,
ignores = [],
}) {
const pluginRules = plugins.map((p) => require(path.join(process.cwd(), p)));
}: Config): Promise<number> {
const pluginRules = plugins.map(
// eslint-disable-next-line @typescript-eslint/no-var-requires
(p) => require(path.join(process.cwd(), p)) as Record<string, Rule>,
);
const allRules = [builtinRules, ...pluginRules].reduce(
(acc, elem) => ({ ...acc, ...elem }),
{},
Expand All @@ -50,35 +57,38 @@ export async function processDatabase({
}`,
);

const ignoreMatchers = ignores.map((i) => (rule, identifier) => {
let ruleMatch;
if (i.rule) {
ruleMatch = rule === i.rule;
} else if (i.rulePattern) {
ruleMatch = new RegExp(i.rulePattern).test(rule);
} else {
throw new Error(
`Ignore object is missing a rule or rulePattern property: ${JSON.stringify(
i,
)}`,
);
}

let identifierMatch;
if (i.identifier) {
identifierMatch = identifier === i.identifier;
} else if (i.identifierPattern) {
identifierMatch = new RegExp(i.identifierPattern).test(identifier);
} else {
throw new Error(
`Ignore object is missing an identifier or identifierPattern property: ${JSON.stringify(
i,
)}`,
);
}

return ruleMatch && identifierMatch;
});
const ignoreMatchers = ignores.map(
(i): IgnoreMatcher =>
(rule, identifier) => {
let ruleMatch: boolean;
if (i.rule) {
ruleMatch = rule === i.rule;
} else if (i.rulePattern) {
ruleMatch = new RegExp(i.rulePattern).test(rule);
} else {
throw new Error(
`Ignore object is missing a rule or rulePattern property: ${JSON.stringify(
i,
)}`,
);
}

let identifierMatch: boolean;
if (i.identifier) {
identifierMatch = identifier === i.identifier;
} else if (i.identifierPattern) {
identifierMatch = new RegExp(i.identifierPattern).test(identifier);
} else {
throw new Error(
`Ignore object is missing an identifier or identifierPattern property: ${JSON.stringify(
i,
)}`,
);
}

return ruleMatch && identifierMatch;
},
);

const report = createReportFunction(consoleReporter, ignoreMatchers);
const extractedSchemas = await extractSchemas(connection, {
Expand Down
3 changes: 3 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export type { default as Config } from "./Config";
export * from "./engine";
export type { default as Rule } from "./Rule";
Comment on lines +1 to +3
Copy link
Sponsor Contributor Author

@orangain orangain Feb 11, 2024

Choose a reason for hiding this comment

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

Should we export the other non-default types such as SchemaConfig or Reporter from the package root?

Copy link
Owner

Choose a reason for hiding this comment

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

I guess. I don't think anyone is creating custom reporters at this point, but if they want to then it would be useful. I don't think this is urgent though.

File renamed without changes.
Loading