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

Arguments after files option should be considered files. #13078

Closed
michaeljota opened this issue Nov 28, 2018 · 7 comments
Closed

Arguments after files option should be considered files. #13078

michaeljota opened this issue Nov 28, 2018 · 7 comments

Comments

@michaeljota
Copy link

Bug Report or Feature Request (mark with an x)

- [ ] bug report -> please search issues before submitting
- [x] feature request

Command (mark with an x)

- [ ] new
- [ ] build
- [ ] serve
- [ ] test
- [ ] e2e
- [ ] generate
- [ ] add
- [ ] update
- [x] lint
- [ ] xi18n
- [ ] run
- [ ] config
- [ ] help
- [ ] version
- [ ] doc

Versions

Angular CLI: 7.1.0
Node: 8.12.0
OS: win32 x64
Angular:
...

Package                      Version
------------------------------------------------------
@angular-devkit/architect    0.11.0
@angular-devkit/core         7.1.0
@angular-devkit/schematics   7.1.0
@schematics/angular          7.1.0
@schematics/update           0.11.0
rxjs                         6.3.3

Repro steps

I'm using lint-staged to run several cli tools when commiting, including linting. As @angular/cli@6.1.9 I was able to use it as:

  "lint-staged": {
    "*.{ts,html}": [
      "ng lint my-app --fix",
      "git add"
    ],
  },

And in only ran on staged files. However, after upgrading to @angular/cli@7.1.0 it throws Unknown option:{// The route of the file that lint-staged passed as an argument}.

The log given by the failure

× ng lint my-app --fix --files found some errors. Please fix them and try committing again.
�[37mLinting "my-app"...�[39m

�[1m�[31mUnknown option: '~~~route\my.component.html'�[39m�[22m
�[1m�[31mUnknown option: '~~~route\my.component.ts'�[39m�[22m
�[1m�[31mUnknown option: '~~~route\.component.spec.ts'�[39m�[22m

Desired functionality

I think Angular CLI should be able to parse additional arguments in this scenario. ng lint --files should handle the arguments that comes from lint-staged.

Mention any other details that might be useful

I research, and find a "fix" in lint-staged#534. I upgrade the version of lint-staged and add the --files argument at the end of the command. Still, this doesn't work, I think because --files is expecting an array of files. lint-staged has an issue 138(open since March 2017, almost two years) requesting for a way to be able to tell how you want to pass the arguments.

@AndreiShostik
Copy link

AndreiShostik commented Sep 23, 2019

I know that feel, bro. once again convinced that angular-cli sucks

tldr;
the only one workable solutions with ng lint is a enumeration of files with --files option: >ng lint --files src/app/app.module.ts --files src/app/shared/shared.module.ts and relative path only!
so better to use tslint directly.


https://angular.io/cli/lint#options - no info about how to use --files
https://github.com/angular/angular-cli/wiki/lint - the same no info

no matter how are you sending files via --files parameter it doesn't work properly at all. I even don't want to spend one more minute to poking around in angular-cli code to find how it exactly should be. it only accepts one file at a time. it uses blobs but as I understood you have exactly a enumeration of files.

  1. >ng lint shows me errors which I need:
ERROR: src/app/app.module.ts:1:10 - " should be '
ERROR: src/app/shared/shared.module.ts:1:20 - " should be '
  1. >ng lint --files src/app/app.module.ts src/app/shared/shared.module.ts - a space between files
An unhandled exception occurred: Project 'src/app/shared/shared.module.ts' does not support the 'lint' target.
See "angular-errors.log" for further details.
  1. >ng lint --files src/app/app.module.ts,src/app/shared/shared.module.ts - comma between files
Linting "project"...
All files pass linting.
  1. >ng lint --files=["src/app/app.module.ts","src/app/shared/shared.module.ts"] - an array
Linting "project"...
All files pass linting.

wtf?

  1. >ng lint project --files "src/app/app.module.ts" "src/app/shared/shared.module.ts" - let's add a project maybe this will work
Unknown option: 'src/app/shared/shared.module.ts'

ugh, c`mon

  1. >ng lint project --files "D:/project/src/app/app.module.ts" --files "D:/project/src/app/shared/shared.module.ts" - maybe project and absolute paths?
Option "files" was already specified with value ["D:/project/src/app/app.module.ts"]. The new value ["D:/project/src/app/app.module.ts","D:/project/src/app/shared/shared.module.ts"] will ove
rride it.
Linting "project"...
An unhandled exception occurred: File "D:\\project\\src\\app\\app.module.ts" is not part of a TypeScript project 'tsconfig.app.json,tsconfig.spec.json,e2e/tsconfig.json'.
See "angular-errors.log" for further details.

ah, f*** off angular-cli...

@headquarters
Copy link

headquarters commented Feb 4, 2020

As @AndreiShostik mentions above, one way is to pass multiple files to the command is via:

ng lint --files relative/path/file1.ts --files relative/path/file2.ts

However, there is another way to do this that is a bit more succinct. TSLint accepts glob patterns for file paths, so the following works as well:

ng lint --files '{relative/path/file1.ts,relative/path/file2.ts}'

Notice the argument passed to --files is a quoted string with curly braces inside {}. Inside the curly braces are comma-separated relative paths.

I think this is a poorly documented CLI command--the feature already exists, so this issue could probably be closed, but ideally the documentation would be updated to clarify what kind of argument is needed for multiple files with examples for globs.

@dgp1130
Copy link
Collaborator

dgp1130 commented Jun 18, 2020

This bug is an unfortunate consequence of our custom argument parser and ambiguity around options with repeated arguments. Fundamentally there is no way to disambiguate:

# Lint `foo.ts` and `bar.ts` file.
ng lint --files foo.ts bar.ts

and

# Lint `my-app-1` project and `foo.ts` file.
ng lint --files foo.ts my-app-1

In our case requiring --files before each repeated argument is how we disambiguate. Other parsers like yargs have more flexible syntax to handle such cases (such as comma delimited or JavaScript array syntax).

We're looking into replacing our custom argument parser with a more standard one specifically due to issues like this one. We shouldn't be in the business of maintaining our own parser and forcing certain conventions onto users. Hopefully once that happens this should become a bit more flexible.

In the meantime, I think the easiest solution to the problem here is to use a function in .lintstagedrc.js per lint-staged/lint-staged#639.

module.exports = {
  "*.ts": (files) => `ng lint ${files.map((file) => `--files ${file}`).join(' ')}`,
};

This will include --files before each file and should generate the right command for you.

It's a little unfortunate that using lint-changed with ng lint doesn't work out of the box. Hopefully future improvements to our argument parsing implementation will make improve support for this particular story.

@dgp1130 dgp1130 removed the needs: discussion On the agenda for team meeting to determine next steps label Jun 18, 2020
@getsnoopy
Copy link

The main problem isn't that it doesn't work without having a --files switch for each file (in an ideal world this is a problem, but it can easily be dealt with using Array.prototype.map, like the examples above show). It's that using multiple --files switches works, yet it still complains that the --files switch was provided multiple times:

$ ng lint "project" "--configuration=test" --fix --files path/to/file.spec.ts --files path/test.ts
Option "files" was already specified with value ["path/to/file.spec.ts"]. The new value ["path/to/file.spec.ts","path/test.ts"] will override it.

ERROR: path/to/file.spec.ts[10, 3]: Use of debugger statements is forbidden
ERROR: path/test.ts[24, 1]: Use of debugger statements is forbidden

Lint errors found in the listed files.

This is stupid. It seems like the only way to fix this currently is to use the brace-syntax as @headquarters pointed out:

$ ng lint "project" "--configuration=test" --fix --files '{path/to/file.spec.ts,path/test.ts}'

ERROR: path/to/file.spec.ts[10, 3]: Use of debugger statements is forbidden
ERROR: path/test.ts[24, 1]: Use of debugger statements is forbidden

Lint errors found in the listed files.

Either the warning message should be removed and the switch renamed to --file to indicate it only takes one at a time, or the switch should be renamed to --file-pattern or the like so that it is clear what it expects. Or at the very least, the documentation should be updated to reflect the fact that --files expects a glob, not a list of files.

@RezaRahmati
Copy link

RezaRahmati commented Jul 21, 2020

module.exports = {
  "*.ts": (files) => `ng lint ${files.map((file) => `--files ${file}`).join(' ')}`,
};

I did this way and this command is running

ng lint --files /Users/rrahmat/projects/distantly/src/app/event/main/event-sponsors/event-sponsors.component.ts --files /Users/rrahmat/projects/distantly/src/app/shared/devextreme-package-module.ts

and I get a new error

An unhandled exception occurred: File "/Users/rrahmat/projects/distantly/Users/rrahmat/projects/distantly/src/app/event/main/event-sponsors/event-sponsors.component.ts" is not part of a TypeScript project 'tsconfig.app.json,tsconfig.spec.json,e2e/tsconfig.json'.

What I see file path is somehow duplicated

Update 1

if I remove the path ran this command manually it's working fine, not sure how to do it in .lintstagedrc.js

ng lint --files /src/app/event/main/event-sponsors/event-sponsors.component.ts --files /src/app/shared/devextreme-package-module.ts

Update 2

I fixed it by having this

module.exports = {
  "*.ts": ( files ) => ( [
    `ng lint ${files.map((file) => `--files ${file.substring(file.indexOf("/src"))}`).join(' ')}`,
    "git add"
  ] ),
};

@alan-agius4
Copy link
Collaborator

Closing as the tslint builder is deprecated.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants