Various QoL with TSLint and Docs (#12655)

hansl · kyliau · commit c513511fe03c · 2018-10-23T11:59:42.000-07:00
* docs: add README files to all root folders

Except benchmark and tools.

* refactor: move rules/ directory to etc/

And fix a linting error that was found while updating tslint usage

* ci: lint errors out if TSLint shows a warning

This is not a warning from the code itself, but from TSLint. For example,
if a rule was missing, TSLint will just ignore the rule, show something on
STDOUT and not fail.

This fixes CI where local rules were missing for unknown reasons. It will
now fail CI when that happens, which is the correct behaviour.
diff --git a/bin/README.md b/bin/README.md
@@ -0,0 +1,17 @@
+# `/bin` Folder
+
+This folder includes binaries that are linked when globally linking this repository.
+
+Each file in this directory follows this pattern:
+
+1. JavaScript only.
+1. Requires `../lib/bootstrap-local.js` to bootstrap TypeScript and Node integration.
+1. Requires `../lib/packages` and use the package metadata to find the binary script for the 
+package the script is bootstrapping.
+1. Call out main, or simply require the file if it has no export.
+
+`devkit-admin` does not follow this pattern as it needs to setup logging and run some localized 
+logic.
+
+In order to add a new script, you should make sure it's in the root `package.json`, so people
+linking this repo get a reference to the script.
diff --git a/docs/README.md b/docs/README.md
@@ -0,0 +1,25 @@
+# `/docs` Folder
+
+This folder is used for all documentation. It contains a number of subfolders with short 
+descriptions here.
+
+## `/docs/design`
+
+Design documents on GitHub, in Markdown format. The number of design documents available is limited.
+
+## `/docs/documentation`
+
+THIS FOLDER IS DEPRECATED AND SHOULD NOT BE UPDATED ANYMORE. Documentation is now available on
+angular.io and can be updated on the main [angular/angular](https://github.com/angular/angular)
+repo.
+
+Markdown files used to publish to the [GitHub Wiki](https://github.com/angular/angular-cli/wiki).
+
+## `/docs/process`
+
+Description of various caretaker and workflow processes.
+
+## `/docs/specifications`
+
+Specifications for support of Angular CLI features. These are meant to be read directly on GitHub
+by developers of libraries who want to integrate with the Angular CLI.
diff --git a/etc/README.md b/etc/README.md
@@ -0,0 +1,3 @@
+# `/etc` Folder
+
+This folder is for files that doesn't fit in other directories in the root folder.
diff --git a/etc/rules/README.md b/etc/rules/README.md
@@ -0,0 +1,3 @@
+# TsLint Rules
+
+This folder contains custom TsLint rules specific to this repository.
diff --git a/etc/rules/Rule.ts b/etc/rules/Rule.ts
diff --git a/etc/rules/defocusRule.ts b/etc/rules/defocusRule.ts
diff --git a/etc/rules/importGroupsRule.ts b/etc/rules/importGroupsRule.ts
diff --git a/etc/rules/noGlobalTslintDisableRule.ts b/etc/rules/noGlobalTslintDisableRule.ts
diff --git a/etc/rules/singleEofLineRule.ts b/etc/rules/singleEofLineRule.ts
diff --git a/etc/rules/tsconfig.json b/etc/rules/tsconfig.json
@@ -3,7 +3,7 @@
   // things faster.
   "extends": "../tsconfig.json",
   "compilerOptions": {
-    "outDir": "../dist/rules",
+    "outDir": "../../dist/etc/rules",
     "baseUrl": ""
   },
   "exclude": [
diff --git a/lib/README.md b/lib/README.md
@@ -0,0 +1,16 @@
+# `/lib` Folder
+
+This folder includes bootstrap code for the various tools included in this repository. Also 
+included is the packages meta-information package in `packages.ts`. This is used to read and 
+understand all the monorepo information (contained in the `.monorepo.json` file, and `package.json` 
+files across the repo).
+
+`bootstrap-local.js` should be included when running files from this repository without compiling
+first. It allows for compiling and loading packages in memory directly from the repo. Not only 
+does the `devkit-admin` scripts use this to include the library, but also all binaries linked 
+locally (like `schematics` and `ng`), when not using the npm published packages. 
+
+`istanbul-local.js` adds global hooks and information to be able to use code coverage with
+`bootstrap-local.js`. Istanbul does not keep the sourcemaps properly in sync if they're available
+in the code, which happens locally when transpiling TypeScript. It is currently used by the 
+`test` script and is included by `bootstrap-local.js` for integration.
diff --git a/packages/README.md b/packages/README.md
@@ -0,0 +1,9 @@
+# `/packages` Folder
+
+This folder is the root of all defined packages in this repository.
+
+Packages that are marked as `private: true` will not be published to NPM. These are limited to the
+`_` subfolder.
+
+This folder includes a directory for every scope in NPM, without the `@` sign. Then one folder 
+per package, which contains the `package.json`.
diff --git a/packages/ngtools/webpack/src/type_checker_messages.ts b/packages/ngtools/webpack/src/type_checker_messages.ts
@@ -42,4 +42,3 @@ export class LogMessage extends TypeCheckerMessage {
     super(MESSAGE_KIND.Log);
   }
 }
-
diff --git a/scripts/lint.ts b/scripts/lint.ts
@@ -20,7 +20,7 @@ const blacklist = [
 
 
 function _buildRules(logger: logging.Logger) {
-  const tsConfigPath = path.join(__dirname, '../rules/tsconfig.json');
+  const tsConfigPath = path.join(__dirname, '../etc/rules/tsconfig.json');
   const configFile = ts.readConfigFile(tsConfigPath, ts.sys.readFile);
 
   const parsedTsConfig = ts.parseJsonConfigFileContent(
@@ -43,7 +43,7 @@ function _buildRules(logger: logging.Logger) {
 }
 
 
-export default function (options: ParsedArgs, logger: logging.Logger) {
+export default async function (options: ParsedArgs, logger: logging.Logger) {
   _buildRules(logger);
 
   const lintOptions: ILinterOptions = {
@@ -55,6 +55,22 @@ export default function (options: ParsedArgs, logger: logging.Logger) {
   const tsLintPath = path.join(__dirname, '../tslint.json');
   const tsLintConfig = Configuration.loadConfigurationFromPath(tsLintPath);
 
+  // Remove comments from the configuration, ie. keys that starts with "//".
+  [...tsLintConfig.rules.keys()]
+    .filter(x => x.startsWith('//'))
+    .forEach(key => tsLintConfig.rules.delete(key));
+
+  // Console is used directly by tslint, and when finding a rule that doesn't exist it's considered
+  // a warning by TSLint but _only_ shown on the console and impossible to see through the API.
+  // In order to see any warnings that happen from TSLint itself, we hook into console.warn() and
+  // record any calls.
+  const oldWarn = console.warn;
+  let warnings = false;
+  console.warn = (...args) => {
+    warnings = true;
+    oldWarn(...args);
+  };
+
   program.getRootFileNames().forEach(fileName => {
     if (blacklist.some(x => x.test(path.relative(process.cwd(), fileName)))) {
       return;
@@ -63,6 +79,8 @@ export default function (options: ParsedArgs, logger: logging.Logger) {
     linter.lint(fileName, ts.sys.readFile(fileName) || '', tsLintConfig);
   });
 
+  console.warn = oldWarn;
+
   const result = linter.getResult();
   const Formatter = findFormatter('codeFrame');
   if (!Formatter) {
@@ -76,10 +94,18 @@ export default function (options: ParsedArgs, logger: logging.Logger) {
     if (result.warningCount > 0) {
       logger.info(`Warnings: ${result.warningCount}`);
     }
-    process.exit(2);
+
+    return 2;
   } else if (result.warningCount > 0) {
     logger.warn(formatter.format(result.failures));
     logger.info(`Warnings: ${result.warningCount}`);
-    process.exit(1);
+
+    return 1;
+  } else if (warnings) {
+    logger.warn('TSLint found warnings, but code is okay. See above.');
+
+    return 1;
   }
+
+  return 0;
 }
diff --git a/tests/README.md b/tests/README.md
@@ -0,0 +1,11 @@
+# `/tests` Folder
+
+Contains all e2e tests and test assets.
+
+## `legacy-cli/`
+
+Contains all assets and all e2e tests from the legacy CLI repo.
+
+## Others
+
+Other folders contain test assets related to the Package namespace folders matching their name.
diff --git a/tsconfig.json b/tsconfig.json
@@ -64,6 +64,7 @@
   "exclude": [
     "bazel-*/**/*",
     "dist/**/*",
+    "etc/cli.angular.io",
     "node_modules/**/*",
     "packages/_/devkit/**/*files/**/*",
     "packages/schematics/*/*/*files/**/*",
diff --git a/tslint.json b/tslint.json
@@ -6,12 +6,12 @@
     "tslint-no-circular-imports"
   ],
   "rulesDirectory": [
-    "dist/rules",
+    "dist/etc/rules",
     "node_modules/tslint-sonarts/lib/rules"
   ],
   "rules": {
     "//03": "==================================================================================================",
-    "//04": "custom rules defined in dist/rules/**",
+    "//04": "custom rules defined in etc/rules/**",
     "defocus": true,
     "import-groups": true,
     "no-global-tslint-disable": true,
@@ -86,7 +86,6 @@
     "no-internal-module": true,
     "no-trailing-whitespace": true,
     "no-unused-expression": true,
-    "no-unused-variable": [true, {"ignore-pattern": "^_"}],
     "no-var-keyword": true,
     "one-line": [
       true,

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+# `/etc` Folder
	`2`	`+`
	`3`	`+This folder is for files that doesn't fit in other directories in the root folder.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+# TsLint Rules`
	`2`	`+`
	`3`	`+This folder contains custom TsLint rules specific to this repository.`