fix: files are not excluded when using user-provided tsconfig (and va…

…rious other fixes and improvements) (#969)

When using a user-provided tsconfig, the `exclude` and `include` options
were incorrectly dropped and instead all `*.ts` files were always
included in the build.

This change also improves on the compile test which previously actually
didn't compile anything.
Also added a bunch of additional configurable options to make many more
common tsconfigs pass the the strict validation.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

src/compiler.ts

@@ -65,7 +65,7 @@ export class Compiler implements Emitter {
   private readonly system: ts.System;
   private readonly compilerHost: ts.CompilerHost;
   private readonly userProvidedTypeScriptConfig: boolean;
-  private readonly tsConfig: TypeScriptConfig;
+  private readonly tsconfig: TypeScriptConfig;
   private rootFiles: string[] = [];
   private readonly configPath: string;
   private readonly projectRoot: string;
@@ -81,7 +81,7 @@ export class Compiler implements Emitter {
     const configFileName = options.typeScriptConfig ?? options.generateTypeScriptConfig ?? 'tsconfig.json';
     this.configPath = path.join(this.projectRoot, configFileName);
     this.userProvidedTypeScriptConfig = Boolean(options.typeScriptConfig);
-    this.tsConfig = this.configureTypeScript();
+    this.tsconfig = this.configureTypeScript();
 
     this.system = {
       ...ts.sys,
@@ -99,7 +99,7 @@ export class Compiler implements Emitter {
         ts.sys.writeFile(path.resolve(this.projectRoot, pth), data, writeByteOrderMark),
     };
 
-    this.compilerHost = ts.createIncrementalCompilerHost(this.tsConfig.compilerOptions, this.system);
+    this.compilerHost = ts.createIncrementalCompilerHost(this.tsconfig.compilerOptions, this.system);
   }
 
   /**
@@ -130,14 +130,14 @@ export class Compiler implements Emitter {
     const host = ts.createWatchCompilerHost(
       this.configPath,
       {
-        ...this.tsConfig.compilerOptions,
+        ...this.tsconfig.compilerOptions,
         noEmitOnError: false,
       },
       this.system,
       ts.createEmitAndSemanticDiagnosticsBuilderProgram,
       opts?.reportDiagnostics,
       opts?.reportWatchStatus,
-      this.tsConfig.watchOptions,
+      this.tsconfig.watchOptions,
     );
     if (!host.getDefaultLibLocation) {
       throw new Error('No default library location was found on the TypeScript compiler host!');
@@ -192,6 +192,7 @@ export class Compiler implements Emitter {
 
       // validate the user provided config
       if (rules !== TypeScriptConfigValidationRuleSet.NONE) {
+        const configName = path.relative(this.projectRoot, this.configPath);
         try {
           const validator = new TypeScriptConfigValidator(rules);
           validator.validate({
@@ -202,12 +203,16 @@ export class Compiler implements Emitter {
         } catch (error: unknown) {
           if (error instanceof ValidationError) {
             utils.logDiagnostic(
-              JsiiDiagnostic.JSII_4000_FAILED_TSCONFIG_VALIDATION.create(undefined, this.configPath, error.violations),
+              JsiiDiagnostic.JSII_4000_FAILED_TSCONFIG_VALIDATION.create(
+                undefined,
+                configName,
+                rules,
+                error.violations,
+              ),
               this.projectRoot,
             );
           }
 
-          const configName = path.relative(this.projectRoot, this.configPath);
           throw new Error(
             `Failed validation of tsconfig "compilerOptions" in "${configName}" against rule set "${rules}"!`,
           );
@@ -246,7 +251,7 @@ export class Compiler implements Emitter {
       throw new Error('No default library location was found on the TypeScript compiler host!');
     }
 
-    const tsconf = this.tsConfig!;
+    const tsconf = this.tsconfig!;
 
     const prog = ts.createIncrementalProgram({
       rootNames: this.rootFiles.concat(_pathOfLibraries(this.compilerHost)),
@@ -399,6 +404,7 @@ export class Compiler implements Emitter {
     return {
       compilerOptions: extended.options,
       watchOptions: extended.watchOptions,
+      include: extended.fileNames,
     };
   }
 
@@ -411,7 +417,7 @@ export class Compiler implements Emitter {
     const commentKey = '_generated_by_jsii_';
     const commentValue = 'Generated by jsii - safe to delete, and ideally should be in .gitignore';
 
-    (this.tsConfig as any)[commentKey] = commentValue;
+    (this.tsconfig as any)[commentKey] = commentValue;
 
     if (fs.existsSync(this.configPath)) {
       const currentConfig = JSON.parse(fs.readFileSync(this.configPath, 'utf-8'));
@@ -423,8 +429,8 @@ export class Compiler implements Emitter {
     }
 
     const outputConfig = {
-      ...this.tsConfig,
-      compilerOptions: convertForJson(this.tsConfig?.compilerOptions),
+      ...this.tsconfig,
+      compilerOptions: convertForJson(this.tsconfig?.compilerOptions),
     };
 
     LOG.debug(`Creating or updating ${chalk.blue(this.configPath)}`);
@@ -488,21 +494,20 @@ export class Compiler implements Emitter {
    * This makes it so that running 'typescript' and running 'jsii' has the same behavior.
    */
   private determineSources(files: string[]): string[] {
-    const ret = new Array<string>();
-
+    // explicitly requested files
     if (files.length > 0) {
-      ret.push(...files);
-    } else {
-      const parseConfigHost = parseConfigHostFromCompilerHost(this.compilerHost);
-      const parsed = ts.parseJsonConfigFileContent(
-        this.tsConfig,
-        parseConfigHost,
-        this.options.projectInfo.projectRoot,
-      );
-      ret.push(...parsed.fileNames);
+      return [...files];
     }
 
-    return ret;
+    // for user provided config we already have parsed the full list of files
+    if (this.userProvidedTypeScriptConfig) {
+      return [...(this.tsconfig.include ?? [])];
+    }
+
+    // finally get the file list for the generated config
+    const parseConfigHost = parseConfigHostFromCompilerHost(this.compilerHost);
+    const parsed = ts.parseJsonConfigFileContent(this.tsconfig, parseConfigHost, this.options.projectInfo.projectRoot);
+    return [...parsed.fileNames];
   }
 
   /**

src/jsii-diagnostic.ts

@@ -432,8 +432,8 @@ export class JsiiDiagnostic implements ts.Diagnostic {
 
   public static readonly JSII_4000_FAILED_TSCONFIG_VALIDATION = Code.error({
     code: 4000,
-    formatter: (config: string, violations: Array<Violation>) => {
-      return `Typescript compiler options in "${config}" are not passing validation, found the following rule violations:\n${violations
+    formatter: (config: string, ruleSet: string, violations: Array<Violation>) => {
+      return `Typescript compiler options in "${config}" are not passing validation against rule set "${ruleSet}", found the following rule violations:\n${violations
         .map((v) => `  - ${v.field}: ${v.message}`)
         .join('\n')}`;
     },

src/main.ts

@@ -123,7 +123,7 @@ const ruleSets: {
               ruleSets,
               '[EXPERIMENTAL] Validate the provided typescript configuration file against a set of rules.',
             ),
-            default: TypeScriptConfigValidationRuleSet.STRICT,
+            defaultDescription: TypeScriptConfigValidationRuleSet.STRICT,
           })
           .option('compress-assembly', {
             group: OPTION_GROUP.JSII,
@@ -159,6 +159,12 @@ const ruleSets: {
 
         configureCategories(projectInfo.diagnostics ?? {});
 
+        const typeScriptConfig = argv.tsconfig ?? projectInfo.packageJson.jsii?.tsconfig;
+        const validateTypeScriptConfig =
+          (argv['validate-tsconfig'] as TypeScriptConfigValidationRuleSet) ??
+          projectInfo.packageJson.jsii?.validateTsconfig ??
+          TypeScriptConfigValidationRuleSet.STRICT;
+
         const compiler = new Compiler({
           projectInfo,
           projectReferences: argv['project-references'],
@@ -167,10 +173,8 @@ const ruleSets: {
           stripDeprecatedAllowListFile: argv['strip-deprecated'],
           addDeprecationWarnings: argv['add-deprecation-warnings'],
           generateTypeScriptConfig: argv['generate-tsconfig'],
-          typeScriptConfig: argv.tsconfig ?? projectInfo.packageJson.jsii?.tsconfig,
-          validateTypeScriptConfig:
-            (argv['validate-tsconfig'] as TypeScriptConfigValidationRuleSet) ??
-            projectInfo.packageJson.jsii?.validateTsConfig,
+          typeScriptConfig,
+          validateTypeScriptConfig,
           compressAssembly: argv['compress-assembly'],
         });
 

src/project-info.ts

@@ -81,7 +81,7 @@ export interface ProjectInfo {
 
   // user-provided tsconfig
   readonly tsconfig?: string;
-  readonly validateTsConfig?: TypeScriptConfigValidationRuleSet;
+  readonly validateTsconfig?: TypeScriptConfigValidationRuleSet;
 }
 
 export interface PackageJson {
@@ -125,7 +125,7 @@ export interface PackageJson {
 
     // Either user-provided config ...
     readonly tsconfig?: string;
-    readonly validateTsConfig?: string;
+    readonly validateTsconfig?: string;
 
     // ... or configure tsc here
     readonly excludeTypescript?: readonly string[];
@@ -270,7 +270,7 @@ export function loadProjectInfo(projectRoot: string): ProjectInfoResult {
 
     // user-provided tsconfig
     tsconfig: pkg.jsii?.tsconfig,
-    validateTsConfig: _validateTsConfigRuleSet(pkg.jsii?.validateTsConfig ?? 'strict'),
+    validateTsconfig: _validateTsconfigRuleSet(pkg.jsii?.validateTsconfig ?? 'strict'),
   };
   return { projectInfo, diagnostics };
 }
@@ -496,13 +496,13 @@ function _validateStability(stability: string | undefined, deprecated: string |
   return stability as spec.Stability;
 }
 
-function _validateTsConfigRuleSet(ruleSet: string): TypeScriptConfigValidationRuleSet | undefined {
+function _validateTsconfigRuleSet(ruleSet: string): TypeScriptConfigValidationRuleSet | undefined {
   if (ruleSet == null) {
     return undefined;
   }
   if (!Object.values(TypeScriptConfigValidationRuleSet).includes(ruleSet as any)) {
     throw new Error(
-      `Invalid validateTsConfig "${ruleSet}", it must be one of ${Object.values(TypeScriptConfigValidationRuleSet).join(
+      `Invalid validateTsconfig "${ruleSet}", it must be one of ${Object.values(TypeScriptConfigValidationRuleSet).join(
         ', ',
       )}`,
     );

src/tsconfig/rulesets/configurable-options.ts

@@ -0,0 +1,22 @@
+import jsiiConfiguredOptions from './jsii-configured-options';
+import { Match, RuleSet } from '../validator';
+
+// A rule set defining all compilerOptions that can be configured by users with or without constraints.
+// These are options jsii doesn't have a particular opinion about
+// This is an internal rule set, that may be used by other rule sets.
+
+const configurableOptions = new RuleSet();
+
+// import all options that are configurable via jsii settings
+configurableOptions.import(jsiiConfiguredOptions);
+
+// options jsii allows to be configured
+configurableOptions.shouldPass('incremental', Match.ANY);
+configurableOptions.shouldPass('noImplicitReturns', Match.ANY);
+configurableOptions.shouldPass('noUnusedLocals', Match.ANY);
+configurableOptions.shouldPass('noUnusedParameters', Match.ANY);
+configurableOptions.shouldPass('resolveJsonModule', Match.ANY);
+configurableOptions.shouldPass('experimentalDecorators', Match.ANY);
+configurableOptions.shouldPass('noFallthroughCasesInSwitch', Match.ANY);
+
+export default configurableOptions;

src/tsconfig/rulesets/deprecated-options.ts

@@ -0,0 +1,8 @@
+import { Match, RuleSet } from '../validator';
+
+// A rule set for deprecated compilerOptions that should not be used with jsii
+// This is an internal rule set, that may be used by other rule sets.
+const deprecatedOptions = new RuleSet();
+deprecatedOptions.shouldPass('prepend', Match.MISSING);
+
+export default deprecatedOptions;

src/tsconfig/rulesets/generated.public.ts

@@ -1,4 +1,4 @@
-import configurable from './configurable';
+import jsiiConfiguredOptions from './jsii-configured-options';
 import { BASE_COMPILER_OPTIONS, convertForJson } from '../compiler-options';
 import { Match, RuleSet, RuleType } from '../validator';
 
@@ -11,7 +11,7 @@ const generated = new RuleSet({
 });
 
 // import all options that are configurable via jsii settings
-generated.import(configurable);
+generated.import(jsiiConfiguredOptions);
 
 // ... and all generated options
 for (const [field, value] of Object.entries(convertForJson(BASE_COMPILER_OPTIONS))) {

src/tsconfig/rulesets/jsii-configured-options.ts

@@ -0,0 +1,20 @@
+import { Match, RuleSet } from '../validator';
+
+// A rule set defining all compilerOptions that are configurable via the jsii field in package.json
+// This is an internal rule set, that may be used by other rule sets.
+// We accept all value for these
+const jsiiConfiguredOptions = new RuleSet();
+jsiiConfiguredOptions.shouldPass('outdir', Match.ANY);
+jsiiConfiguredOptions.shouldPass('rootDir', Match.ANY);
+jsiiConfiguredOptions.shouldPass('forceConsistentCasingInFileNames', Match.ANY);
+jsiiConfiguredOptions.shouldPass('declarationMap', Match.ANY);
+jsiiConfiguredOptions.shouldPass('inlineSourceMap', Match.ANY);
+jsiiConfiguredOptions.shouldPass('inlineSources', Match.ANY);
+jsiiConfiguredOptions.shouldPass('sourceMap', Match.ANY);
+jsiiConfiguredOptions.shouldPass('types', Match.ANY);
+jsiiConfiguredOptions.shouldPass('baseUrl', Match.ANY);
+jsiiConfiguredOptions.shouldPass('paths', Match.ANY);
+jsiiConfiguredOptions.shouldPass('composite', Match.ANY); // configured via projectReferences
+jsiiConfiguredOptions.shouldPass('tsBuildInfoFile', Match.ANY);
+
+export default jsiiConfiguredOptions;

src/tsconfig/rulesets/minimal.public.ts

@@ -1,10 +1,10 @@
-import incompatible from './incompatible-options';
+import incompatibleOptions from './incompatible-options';
 import { RuleSet } from '../validator';
 
 // The public rule set used for the "minimal" tsconfig validation setting
 // To goal of this rule set is to only prevent obvious misconfigurations,
 // while leaving everything else up to the user.
 const minimal = new RuleSet();
-minimal.import(incompatible);
+minimal.import(incompatibleOptions);
 
 export default minimal;

src/tsconfig/rulesets/strict-family-options.ts

@@ -0,0 +1,17 @@
+import { Match, RuleSet } from '../validator';
+
+// A rule set for the compilerOptions of the strict family.
+// The rule set enforces strict, but allows the defining options that are implied by strict
+
+const strictFamilyOptions = new RuleSet();
+strictFamilyOptions.shouldPass('strict', Match.eq(true));
+strictFamilyOptions.shouldPass('alwaysStrict', Match.optional(Match.eq(true)));
+strictFamilyOptions.shouldPass('noImplicitAny', Match.optional(Match.eq(true)));
+strictFamilyOptions.shouldPass('noImplicitThis', Match.optional(Match.eq(true)));
+strictFamilyOptions.shouldPass('strictBindCallApply', Match.optional(Match.eq(true)));
+strictFamilyOptions.shouldPass('strictFunctionTypes', Match.optional(Match.eq(true)));
+strictFamilyOptions.shouldPass('strictNullChecks', Match.optional(Match.eq(true)));
+strictFamilyOptions.shouldPass('strictPropertyInitialization', Match.optional(Match.eq(true)));
+strictFamilyOptions.shouldPass('useUnknownInCatchVariables', Match.optional(Match.eq(true)));
+
+export default strictFamilyOptions;

src/tsconfig/rulesets/strict.public.ts

@@ -1,5 +1,7 @@
-import configurable from './configurable';
-import incompatible from './incompatible-options';
+import configurableOptions from './configurable-options';
+import deprecatedOptions from './deprecated-options';
+import incompatibleOptions from './incompatible-options';
+import strictFamilyOptions from './strict-family-options';
 import { Match, RuleSet, RuleType } from '../validator';
 
 // The public rule set used for the "strict" tsconfig validation setting.
@@ -11,10 +13,13 @@ const strict = new RuleSet({
 });
 
 // import all options that are configurable
-strict.import(configurable);
+strict.import(configurableOptions);
 
 // import all options that are definitely incompatible
-strict.import(incompatible);
+strict.import(incompatibleOptions);
+
+// strict family options
+strict.import(strictFamilyOptions);
 
 // Best practice rules
 strict.shouldPass('target', Match.eq('es2022')); // node18
@@ -23,10 +28,10 @@ strict.shouldPass('module', Match.oneOf('node16', 'commonjs'));
 strict.shouldPass('moduleResolution', Match.optional(Match.oneOf('node', 'node16')));
 strict.shouldPass('esModuleInterop', Match.TRUE);
 strict.shouldPass('skipLibCheck', Match.TRUE);
-strict.shouldPass('strict', Match.eq(true));
-strict.shouldPass('incremental', Match.ANY);
+strict.shouldPass('stripInternal', Match.optional(Match.FALSE));
+strict.shouldPass('noEmitOnError', Match.TRUE);
 
 // Deprecated ts options that should not be used with jsii
-strict.shouldPass('prepend', Match.MISSING);
+strict.import(deprecatedOptions);
 
 export default strict;