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

feat: Add full warnings list, dont ouput warning if disabled #1838

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
41 changes: 27 additions & 14 deletions src/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ export class Assembler implements Emitter {
}
const readme = _loadReadme.call(this);
if (readme == null) {
this._diagnostics.push(JsiiDiagnostic.JSII_0003_MISSING_README.createDetached());
if (enabledWarnings['metadata/missing-readme']) {
this._diagnostics.push(JsiiDiagnostic.JSII_0003_MISSING_README.createDetached());
}
}
const docs = _loadDocs.call(this);

Expand Down Expand Up @@ -370,7 +372,10 @@ export class Assembler implements Emitter {
// since we are exposing a type of this assembly in this module's public API,
// we expect it to appear as a peer dependency instead of a normal dependency.
if (assembly) {
if (!(assembly.name in this.projectInfo.peerDependencies)) {
if (
!(assembly.name in this.projectInfo.peerDependencies) &&
enabledWarnings['metadata/missing-peer-dependency']
) {
this._diagnostics.push(
JsiiDiagnostic.JSII_0005_MISSING_PEER_DEPENDENCY.create(
referencingNode!, // Cheating here for now, until the referencingNode can be made required
Expand Down Expand Up @@ -1637,7 +1642,7 @@ export class Assembler implements Emitter {
const params = getReferencedDocParams(methodSym);
const actualNames = new Set((method.parameters ?? []).map((p) => p.name));
for (const param of params) {
if (!actualNames.has(param)) {
if (!actualNames.has(param) && enabledWarnings['documentation/non-existent-parameter']) {
this._diagnostics.push(
JsiiDiagnostic.JSII_7000_NON_EXISTENT_PARAMETER.create(
methodSym.valueDeclaration ?? methodSym.declarations?.[0],
Expand Down Expand Up @@ -1871,7 +1876,10 @@ export class Assembler implements Emitter {
return;
}

if (type.name === Case.pascal(symbol.name)) {
if (
type.name === Case.pascal(symbol.name) &&
enabledWarnings['language-compatibility/member-name-conflicts-with-type-name']
) {
this._diagnostics.push(
JsiiDiagnostic.JSII_5019_MEMBER_TYPE_NAME_CONFLICT.create(
declaration.name,
Expand Down Expand Up @@ -1952,15 +1960,17 @@ export class Assembler implements Emitter {
}

private _warnAboutReservedWords(symbol: ts.Symbol) {
if (!enabledWarnings['reserved-word']) {
return;
}

const reservingLanguages = isReservedName(symbol.name);
if (reservingLanguages) {
this._diagnostics.push(
JsiiDiagnostic.JSII_5018_RESERVED_WORD.create(_nameOrDeclarationNode(symbol), symbol.name, reservingLanguages),
);
if (enabledWarnings['language-compatibility/reserved-word']) {
const reservingLanguages = isReservedName(symbol.name);
if (reservingLanguages) {
this._diagnostics.push(
JsiiDiagnostic.JSII_5018_RESERVED_WORD.create(
_nameOrDeclarationNode(symbol),
symbol.name,
reservingLanguages,
),
);
}
}
}

Expand Down Expand Up @@ -1990,7 +2000,10 @@ export class Assembler implements Emitter {
| ts.AccessorDeclaration
| ts.ParameterPropertyDeclaration;

if (type.name === Case.pascal(symbol.name)) {
if (
type.name === Case.pascal(symbol.name) &&
enabledWarnings['language-compatibility/member-name-conflicts-with-type-name']
) {
this._diagnostics.push(
JsiiDiagnostic.JSII_5019_MEMBER_TYPE_NAME_CONFLICT.create(
signature.name,
Expand Down
6 changes: 5 additions & 1 deletion src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { BASE_COMPILER_OPTIONS, convertForJson } from './tsconfig/compiler-optio
import { TypeScriptConfigValidator } from './tsconfig/tsconfig-validator';
import { ValidationError } from './tsconfig/validator';
import * as utils from './utils';
import { enabledWarnings } from './warnings';

const LOG = log4js.getLogger('jsii/compiler');
export const DIAGNOSTICS = 'diagnostics';
Expand Down Expand Up @@ -182,7 +183,10 @@ export class Compiler implements Emitter {

// emit a warning if validation is disabled
const rules = this.options.validateTypeScriptConfig ?? TypeScriptConfigValidationRuleSet.NONE;
if (rules === TypeScriptConfigValidationRuleSet.NONE) {
if (
rules === TypeScriptConfigValidationRuleSet.NONE &&
enabledWarnings['typescript-config/disabled-tsconfig-validation']
) {
utils.logDiagnostic(
JsiiDiagnostic.JSII_4009_DISABLED_TSCONFIG_VALIDATION.create(undefined, this.configPath),
this.projectRoot,
Expand Down
7 changes: 5 additions & 2 deletions src/directives.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as ts from 'typescript';
import { JsiiDiagnostic } from './jsii-diagnostic';
import { enabledWarnings } from './warnings';

/**
* TSDoc-style directives that can be attached to a symbol.
Expand Down Expand Up @@ -40,7 +41,7 @@ export class Directives {
break;
case 'jsii':
const comments = getComments(tag);
if (comments.length === 0) {
if (comments.length === 0 && enabledWarnings['jsii-directive/missing-argument']) {
onDiagnostic(JsiiDiagnostic.JSII_2000_MISSING_DIRECTIVE_ARGUMENT.create(tag));
continue;
}
Expand All @@ -50,7 +51,9 @@ export class Directives {
this.ignore ??= jsdocNode;
break;
default:
onDiagnostic(JsiiDiagnostic.JSII_2999_UNKNOWN_DIRECTIVE.create(jsdocNode, text));
if (enabledWarnings['jsii-directive/unknown']) {
onDiagnostic(JsiiDiagnostic.JSII_2999_UNKNOWN_DIRECTIVE.create(jsdocNode, text));
}
break;
}
}
Expand Down
17 changes: 3 additions & 14 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import { emitSupportPolicyInformation } from './support';
import { TypeScriptConfigValidationRuleSet } from './tsconfig';
import * as utils from './utils';
import { VERSION } from './version';
import { enabledWarnings } from './warnings';

const warningTypes = Object.keys(enabledWarnings);
import { enabledWarnings, silenceWarnings } from './warnings';

function choiceWithDesc(
choices: { [choice: string]: string },
Expand Down Expand Up @@ -91,7 +89,7 @@ const ruleSets: {
group: OPTION_GROUP.JSII,
type: 'array',
default: [],
desc: `List of warnings to silence (warnings: ${warningTypes.join(',')})`,
desc: `List of warnings to silence (warnings: ${Object.keys(enabledWarnings).join(',')})`,
})
.option('strip-deprecated', {
group: OPTION_GROUP.JSII,
Expand Down Expand Up @@ -149,16 +147,7 @@ const ruleSets: {

const { projectInfo, diagnostics: projectInfoDiagnostics } = loadProjectInfo(projectRoot);

// disable all silenced warnings
for (const key of argv['silence-warnings']) {
if (!(key in enabledWarnings)) {
throw new utils.JsiiError(
`Unknown warning type ${key as any}. Must be one of: ${warningTypes.join(', ')}`,
);
}

enabledWarnings[key] = false;
}
silenceWarnings(argv['silence-warnings'] as string[]);

configureCategories(projectInfo.diagnostics ?? {});

Expand Down
23 changes: 13 additions & 10 deletions src/project-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { findDependencyDirectory } from './common/find-utils';
import { JsiiDiagnostic } from './jsii-diagnostic';
import { TypeScriptConfigValidationRuleSet } from './tsconfig';
import { JsiiError, parsePerson, parseRepository } from './utils';
import { enabledWarnings } from './warnings';

// eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports
const spdx: Set<string> = require('spdx-license-list/simple');
Expand Down Expand Up @@ -178,16 +179,18 @@ export function loadProjectInfo(projectRoot: string): ProjectInfoResult {
const range = new semver.Range(_resolveVersion(rng as string, projectRoot).version);
const minVersion = semver.minVersion(range)?.raw;

if (!(name in devDependencies) || devDependencies[name] !== `${minVersion}`) {
diagnostics.push(
JsiiDiagnostic.JSII_0006_MISSING_DEV_DEPENDENCY.createDetached(
name,
`${rng as any}`,
`${minVersion}`,
`${devDependencies[name]}`,
),
);
continue;
if (enabledWarnings['metadata/missing-dev-dependency']) {
if (!(name in devDependencies) || devDependencies[name] !== `${minVersion}`) {
diagnostics.push(
JsiiDiagnostic.JSII_0006_MISSING_DEV_DEPENDENCY.createDetached(
name,
`${rng as any}`,
`${minVersion}`,
`${devDependencies[name]}`,
),
);
continue;
}
}
}

Expand Down
36 changes: 34 additions & 2 deletions src/warnings.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,39 @@
import { JsiiError } from './utils';

/**
* Indicates which warnings are currently enabled. By default all warnings are
* enabled, and can be silenced through the --silence-warning option.
*/
export const enabledWarnings: { [name: string]: boolean } = {
'reserved-word': true,
export const enabledWarnings = {
'metadata/missing-readme': true,
'metadata/missing-peer-dependency': true,
'metadata/missing-dev-dependency': true,
'jsii-directive/missing-argument': true,
'jsii-directive/struct-on-non-interface': true,
'jsii-directive/unknown': true,
'typescript-config/disabled-tsconfig-validation': true,
'language-compatibility/reserved-word': true,
'language-compatibility/member-name-conflicts-with-type-name': true,
'documentation/non-existent-parameter': true,
};

export const silenceWarnings = (warnings: string[]): void => {
const legacyWarningKeyReplacement: { [key: string]: string } = {
'reserved-word': 'language-compatibility/reserved-word',
};
const legacyWarningKeys = Object.keys(legacyWarningKeyReplacement);

for (const key of warnings) {
if (!(key in enabledWarnings) && !legacyWarningKeys.includes(key)) {
throw new JsiiError(
`Unknown warning type ${key as any}. Must be one of: ${Object.keys(enabledWarnings).join(', ')}`,
);
}

if (legacyWarningKeys.includes(key)) {
enabledWarnings[legacyWarningKeyReplacement[key] as keyof typeof enabledWarnings] = false;
} else {
enabledWarnings[key as keyof typeof enabledWarnings] = false;
}
}
};
36 changes: 36 additions & 0 deletions test/warnings.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { DiagnosticCategory } from 'typescript';
import { Code, JsiiDiagnostic } from '../src/jsii-diagnostic';
import { JsiiError } from '../src/utils';
import { enabledWarnings, silenceWarnings } from '../src/warnings';

describe('enabledWarnings', () => {
test('contains all available Jsii warnings', () => {
const definedWarnings = Object.keys(JsiiDiagnostic).reduce((warnings, warningKey) => {
const code = JsiiDiagnostic[warningKey as keyof typeof JsiiDiagnostic];
if (code instanceof Code && code.category === DiagnosticCategory.Warning) {
warnings[code.name] = true;
}
return warnings;
}, {} as { [name: string]: boolean });

expect(enabledWarnings).toStrictEqual(definedWarnings);
});
});

describe('silenceWarnings', () => {
test('sets enabledWarnings key to false', () => {
expect(enabledWarnings['metadata/missing-readme']).toBe(true);
silenceWarnings(['metadata/missing-readme']);
expect(enabledWarnings['metadata/missing-readme']).toBe(false);
});

test('translates legacy key to current Code.name', () => {
expect(enabledWarnings['language-compatibility/reserved-word']).toBe(true);
silenceWarnings(['reserved-word']);
expect(enabledWarnings['language-compatibility/reserved-word']).toBe(false);
});

test('throws when key is not valid', () => {
expect(() => silenceWarnings(['invalid-warning'])).toThrow(JsiiError);
});
});