New: TypeScript config parser #847

sarvaje · 2018-03-01T18:28:21Z

New: typescript-config-is-valid rule.

Pull request checklist

Make sure you:

Signed the Contributor License Agreement
Followed the commit message guidelines

For non-trivial changes, please make sure you also:

Added/Updated related documentation.
Added/Updated related tests.

Short description of the change(s)

molant · 2018-03-02T17:53:51Z

packages/parser-typescript-config/README.md

+
+## Events emitted
+
+This `parser` emits the following events the events:


the following events:

I would also explain how they can import them in their project.

molant · 2018-03-02T17:55:23Z

packages/parser-typescript-config/README.md

+  * `resource`: the parsed resource.
+  * `errors`: all the errors that the schama validator returns.
+
+* `notfound::typescript-config`. This event doesn't containt anything else.


Maybe explain this event is sent at the end if no TypeScript configuration has been found?

molant · 2018-03-02T17:55:50Z

packages/parser-typescript-config/README.md

+wich contains the folloing information:
+
+  * `resource`: the parsed resource.
+  * `error`: the error emited parsing the configuration file.


How does an error look like?

molant · 2018-03-02T17:59:08Z

packages/parser-typescript-config/src/types.ts

+    LF = 'LF'
+}
+
+export type TypeScriptPaths = {


Missing jsdoc in a few exports in this file

molant · 2018-03-02T18:00:33Z

packages/parser-typescript-config/src/typescript-config.ts

+    }
+
+    private async validateSchema(config: TypeScriptConfig, resource: string) {
+        // ajv is lower case to be able to get the types.


I don't get this comment. Why exactly can't we do:

import * as Ajv from 'ajv'; const x: Ajv.Ajv = new Ajv({});

molant · 2018-03-02T18:13:04Z

packages/rule-typescript-config-is-valid/src/rule.ts

+        const invalidSchema = async (fetchEnd: TypeScriptConfigInvalidSchema) => {
+            const { errors, resource } = fetchEnd;
+
+            debug(`Validating rule typescript-config-is-valid`);


Maybe something like:

schema is invalid

debug should already prefix where the message is coming from

molant · 2018-03-02T18:15:07Z

packages/rule-typescript-config-is-valid/tests/fixtures/noconfig/CHANGELOG.md

@@ -0,0 +1,3 @@
+# 1.0.0 (February 8, 2018)


It's a bit weird to have this as a fixture. Put other content that explains the purpose of this file.

molant · 2018-03-02T18:15:28Z

packages/rule-typescript-config-is-valid/tests/tests.ts

+const ruleName = getRuleName(__dirname);
+
+/*
+ * You should test for cases where the rule passes and doesn't.


Remove this comment

molant · 2018-03-02T18:15:56Z

packages/rule-typescript-config-is-valid/tests/tests.ts

+    },
+    {
+        dir: path.join(__dirname, 'fixtures', 'noconfig'),
+        name: `If there is no config files, it should pass`


If there is no config file

molant · 2018-03-02T18:17:33Z

packages/sonarwhal/docs/user-guide/rules/index.md

@@ -18,6 +18,7 @@
 * [`html-checker`](../../../../rule-html-checker/README.md)
 * [`meta-viewport`](../../../../rule-meta-viewport/README.md)
 * [`no-friendly-error-pages`](../../../../rule-no-friendly-error-pages/README.md)
+* [`typescript-config-is-valid`](typescript-config-is-valid.md)


I'd create a new section Development for this kind of rules.

Also kind of related webhintio/webhintio.github.io#391

alrra · 2018-03-02T18:26:24Z

packages/parser-typescript-config/README.md

@@ -0,0 +1,42 @@
+# Parser typescript-config (`@sonarwhal/parser-typescript-config`)
+
+The `typescript-config` parser allow the user analyze the


allow_s_ the user to analyze

alrra · 2018-03-02T18:28:24Z

packages/parser-typescript-config/src/schema/tsConfigSchema.json

@@ -0,0 +1,473 @@
+{


@sarvaje, @molant l presume the release script should also update this file?

This file is not exactly a copy/past of the original one. I'm adding a couple of "additionalProperties": false.

alrra · 2018-03-02T18:28:54Z

packages/parser-typescript-config/src/typescript-config.ts

+import { Sonarwhal } from 'sonarwhal/dist/src/lib/sonarwhal';
+import { loadJSONFile } from 'sonarwhal/dist/src/lib/utils/misc';
+
+


Remove extra empty line.

alrra · 2018-03-02T18:29:08Z

packages/parser-typescript-config/src/typescript-config.ts

+    public constructor(sonarwhal: Sonarwhal) {
+        super(sonarwhal);
+
+


Remove extra empty line.

alrra · 2018-03-02T18:30:54Z

packages/rule-typescript-config-is-valid/LICENSE.txt

@@ -0,0 +1,201 @@
+                                 Apache License


alrra · 2018-03-02T18:31:26Z

packages/rule-typescript-config-is-valid/README.md

@@ -0,0 +1,56 @@
+# typescript-config-is-valid (`typescript-config-is-valid`)


This should contain the package name :@sonarwhal/typescript-config-is-valid.

sarvaje · 2018-03-02T19:43:54Z

@molant @alrra done!!

sarvaje · 2018-03-03T00:26:31Z

@molant @alrra I have updated the way we run the test for the local connector. Now there is a new method testLocalRule in the rule-runner .

alrra · 2018-03-05T01:15:25Z

packages/parser-typescript-config/README.md

+
+```json
+{
+    "parsers": ["typescript-config"]


I think we should provide more context here. e.g.:

{ ... "parsers": ["typescript-config"], "rules": { ... }, ... }

alrra · 2018-03-05T01:16:30Z

packages/parser-typescript-config/package.json

+  "private": true,
+  "repository": "sonarwhal/sonarwhal",
+  "scripts": {
+    "build": "npm run clean && npm-run-all build:*",


Add "build-release": "npm run clean && npm run build:assets && tsc --inlineSourceMap false --removeComments true",.

alrra · 2018-03-05T01:20:50Z

packages/rule-typescript-config/package.json

+    "extends": "../../../../.nycrc"
+  },
+  "peerDependencies": {
+    "sonarwhal": "^0.27.0"


@sonarwhal/parser-typescript-config ?

sarvaje · 2018-03-05T17:08:36Z

@alrra done!

sarvaje · 2018-03-06T20:46:40Z

@molant done!

alrra

@sarvaje Can you also add a .npmrc file? Thanks!

alrra · 2018-03-07T11:50:26Z

packages/parser-typescript-config/package.json

+    "clean": "rimraf dist",
+    "lint": "npm-run-all lint:*",
+    "lint:js": "eslint . --cache --ext js --ext md --ext ts --ignore-path ../../.eslintignore --report-unused-disable-directives",
+    "lint:md": "markdownlint *.md",


"lint:md": "markdownlint --ignore CHANGELOG.md *.md",

sarvaje · 2018-03-07T16:04:42Z

@alrra done!

alrra · 2018-03-07T23:12:27Z

packages/parser-typescript-config/README.md

+To use it you will have to install it via `npm`:
+
+```bash
+npm install @sonarwhal/parser-typescript-config


--save-dev ?

I don't think so. you have to install the package to use the parser.

I was referring to suggesting:

npm install --save-dev @sonarwhal/parser-typescript-config

As in, suggest to users to install it as a dev dependency.

but why as a dev dependency? I don't follow you :s

sonarwhal and related package should be used as part of the development process, so they should be used as dev dependencies not dependencies.

Doing

npm install @sonarwhal/parser-typescript-config

will install @sonarwhal/parser-typescript-config as a dependency not as a devDependency, because npm install @sonarwhal/parser-typescript-config is the same as npm install --save @sonarwhal/parser-typescript-config

From https://docs.npmjs.com/cli/install#description:

By default, npm install will install all modules listed as dependencies in package.json.

Maybe the confusion is because I missed -g but if you have sonarwhal installed globally and you want to use this parser, you have to install the package normally, not as a dev dependency.

alrra · 2018-03-07T23:13:25Z

packages/parser-typescript-config/README.md

+  which contains the following information:
+
+  * `resource`: the parsed resource.
+  * `config`: an object with a valid the configuration.


Drop the.

alrra · 2018-03-07T23:16:33Z

packages/parser-typescript-config/README.md

+  * `config`: an object with a valid the configuration.
+
+* `invalid-json::typescript-config`, of type `TypeScriptConfigInvalid`
+  wich contains the folloing information:


alrra · 2018-03-07T23:16:36Z

packages/parser-typescript-config/README.md

+  * `error`: the error emited parsing the configuration file.
+
+* `invalid-schema::typescript-config`, of type `TypeScriptConfigInvalidSchema`
+  wich contains the following information:


sarvaje · 2018-03-08T00:11:03Z

@alrra done!

New: `typescript-config-is-valid` rule.

alrra · 2018-03-08T23:06:28Z

packages/parser-typescript-config/README.md

+
+This parser detect if a TypeScript configuration file is present in your
+project, checking the name of the file (i.e. `tsconfig.json`,
+`tsconfig.developement.json`). This parser just read your config file and


...sconfig.developement.json`), reads it, and validates the content. ?

This parser detects if a

alrra · 2018-03-08T23:09:18Z

packages/rule-typescript-config/README.md

@@ -0,0 +1,58 @@
+# typescript-config/is-valid (`@sonarwhal/rule-typescript-config`)
+
+`typescript-config/is-valid` warns again providing an


qzhou1607-zz · 2018-03-08T23:45:02Z

packages/parser-typescript-config/src/index.ts

@@ -0,0 +1 @@
+module.exports = { javascript: require('./typescript-config') };


I think you meant typescript-config here.

good catch!

sarvaje · 2018-03-08T23:53:17Z

@molant, @alrra, @qzhou1607 done!

sarvaje requested review from alrra, austinkelleher and molant as code owners March 1, 2018 18:28

sarvaje mentioned this pull request Mar 1, 2018

New: TypeScript config parser. #802

Closed

4 tasks

sarvaje added the status:work-in-progress label Mar 1, 2018

molant requested changes Mar 2, 2018

View reviewed changes

molant mentioned this pull request Mar 2, 2018

New: Babel config parser #808

Merged

4 tasks

alrra reviewed Mar 2, 2018

View reviewed changes

sarvaje removed the status:work-in-progress label Mar 3, 2018

alrra reviewed Mar 5, 2018

View reviewed changes

sarvaje force-pushed the parser-typescript branch from 400ccda to b26f278 Compare March 5, 2018 17:08

alrra force-pushed the master branch 3 times, most recently from 0fec684 to a0573d2 Compare March 6, 2018 10:00

sarvaje force-pushed the parser-typescript branch from b26f278 to 5e8841e Compare March 6, 2018 19:38

alrra force-pushed the master branch 2 times, most recently from bd5402c to 7aeb1f0 Compare March 6, 2018 20:38

sarvaje force-pushed the parser-typescript branch from 5e8841e to 32d12cc Compare March 6, 2018 23:13

alrra reviewed Mar 7, 2018

View reviewed changes

sarvaje force-pushed the parser-typescript branch 2 times, most recently from f832056 to a80c20e Compare March 7, 2018 22:29

alrra reviewed Mar 7, 2018

View reviewed changes

alrra force-pushed the master branch from ce76f93 to e3b097b Compare March 8, 2018 17:04

sarvaje force-pushed the parser-typescript branch 2 times, most recently from 9d3b614 to b0d880e Compare March 8, 2018 22:14

molant approved these changes Mar 8, 2018

View reviewed changes

sarvaje added 17 commits March 8, 2018 15:03

New: TypeScript config parser.

68e7116

New: `typescript-config-is-valid` rule.

Fix: Fix parser and rule and tests

1687b47

Fix: Add tests and increase coverage

26eccd8

Fix: Address PR feedback

70dedd5

Fix: Refactor and more

2b8b0b7

Fix: Address PR Feedback

a4607f6

Chore: Move validation back to the parser

cbf6d0a

New: Add .npmrc

d6e28db

Fix: Fix package.json

6537805

Chore: Remove old comment

1224ac9

Fix: Fix resource loader

027db16

Fix: Address feedback

4821277

Fix: Address feedback

62b8d52

Fix: Address PR Feedback

32bcd0f

Fix: Refactor error message generator

3607971

Fix: Refactor a variable name

612794d

Fix: Fix sonarwhal version

e25f451

sarvaje force-pushed the parser-typescript branch from 0125603 to e25f451 Compare March 8, 2018 23:04

alrra reviewed Mar 8, 2018

View reviewed changes

qzhou1607-zz reviewed Mar 8, 2018

View reviewed changes

sarvaje added 2 commits March 8, 2018 15:46

Fix: Fix some typos

ce2a5e5

Fix: Address feedback

b1bf266

alrra closed this in b95cc9c Mar 9, 2018

sarvaje deleted the parser-typescript branch March 23, 2018 15:32

This was referenced Jan 12, 2022

[Snyk] Security upgrade shelljs from 0.7.8 to 0.8.5 zelopy/sonar#37

Open

[Snyk] Security upgrade shelljs from 0.7.8 to 0.8.5 magnologan/sonar#18

Open


		## Events emitted

		This `parser` emits the following events the events:

		@@ -0,0 +1,42 @@
		# Parser typescript-config (`@sonarwhal/parser-typescript-config`)

		The `typescript-config` parser allow the user analyze the

		import { Sonarwhal } from 'sonarwhal/dist/src/lib/sonarwhal';
		import { loadJSONFile } from 'sonarwhal/dist/src/lib/utils/misc';

		@@ -0,0 +1,56 @@
		# typescript-config-is-valid (`typescript-config-is-valid`)

		@@ -0,0 +1,58 @@
		# typescript-config/is-valid (`@sonarwhal/rule-typescript-config`)

		`typescript-config/is-valid` warns again providing an

		@@ -0,0 +1 @@
		module.exports = { javascript: require('./typescript-config') };

New: TypeScript config parser #847

New: TypeScript config parser #847

Conversation

sarvaje commented Mar 1, 2018 • edited Loading

Pull request checklist

Short description of the change(s)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarvaje commented Mar 2, 2018

sarvaje commented Mar 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarvaje commented Mar 5, 2018

sarvaje commented Mar 6, 2018

alrra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarvaje commented Mar 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alrra Mar 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alrra Mar 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarvaje commented Mar 8, 2018

Choose a reason for hiding this comment

molant Mar 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sarvaje commented Mar 8, 2018

sarvaje commented Mar 1, 2018 •

edited

Loading

alrra Mar 7, 2018 •

edited

Loading

alrra Mar 7, 2018 •

edited

Loading

molant Mar 8, 2018 •

edited

Loading