Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

strange behavior when installing via yarn@1.1.0, e.g. whitespace "check-separator" in JSDoc #3251

Closed
johncrim opened this issue Sep 27, 2017 · 25 comments
Labels

Comments

@johncrim
Copy link

johncrim commented Sep 27, 2017

Bug Report

  • TSLint version: 5.7.0
  • TypeScript version: 2.3.4
  • Running TSLint via: (pick one) CLI / Node.js API / VSCode / grunt-tslint / Atom / Visual Studio / etc
    Using ng lint

TypeScript code being linted

class TsLintBug {

  /**
   * Adds x and y.
   */
  public add = (x: number, y: number): number => {
    return x + y;
  };

  /**
   * Subtracts y from x.
   * @returns {number}
   */
  public subtract = (x: number, y: number): number => {
    return x + y;
  };

}

with tslint.json configuration:

    "whitespace": [
      true,
      "check-branch",
      "check-decl",
      "check-operator",
      "check-separator",
      "check-type"
    ],

Actual behavior

ERROR: C:/src/.../TsLintBug.ts[10, 3]: missing whitespace

The linter wants the jsdoc comment start to be modified from /** to /* *. Which fixes the issue, but breaks the jsdoc.

Other workaround which "fix" this tslint error:

  1. Removing the trailing ; on the add method.
  2. Removing the @returns line in the second jsdoc.

Note that if you put a @returns line on the first jsdoc, and remove it from the second jsdoc, the error goes away.

Removing the "check-separator" line from tslint.json also makes the error go away (took me a while to figure that one out).

Expected behavior

No tslint errors.

@ajafff
Copy link
Contributor

ajafff commented Sep 27, 2017

I tried to reproduce this with typescript@2.3.4 and 2.5.2
I didn't see any error

Can you reproduce this without any third party tool, i.e. running tslint -c tslint.json tslintbug.ts directly from command line?

@agxs
Copy link

agxs commented Sep 27, 2017

I have the same issue after purging my node_modules and reinstalling this afternoon. Removing check-separator let the lint pass, not ideal though.

tslint 5.6.0
typescript 2.3.4

Edit:

As mentioned below, if I force yarn to be v1.0.2 and reinstall my node_modules then linting completes with no errors.

@ajafff
Copy link
Contributor

ajafff commented Sep 27, 2017

#3244 (comment) indicates that there might be a problem when using yarn 1.1.0
Can you confirm that you used yarn to install tslint?

@johncrim
Copy link
Author

johncrim commented Sep 27, 2017

Yes - I did use yarn to install tslint. I will try to reproduce using both npm and yarn, with no 3rd party tools.

@nweldev
Copy link

nweldev commented Sep 28, 2017

I had the same issue using yarn 1.1.0 but not with yarn 1.0.2 ... This seems to be a yarn issue.

@PowerKiKi
Copy link

Given the following files:

tslint.json

{
  "rules": {
    "whitespace": [
      true,
      "check-separator"
    ]
  } 
}

bug.ts

class TsLintBug {

  /**
   * Adds x and y.
   */
  public add = (x: number, y: number): number => {
    return x + y;
  };

  /**
   * Subtracts y from x.
   * @returns {number}
   */
  public subtract = (x: number, y: number): number => {
    return x + y;
  };

}

Then I can not reproduce with either npm 5.4.2 or yarn 1.1.0:

$ rm -rf package.json package-lock.json yarn.lock node_modules/ && npm --version && npm install typescript tslint && npm list && ./node_modules/.bin/tslint --config tslint.json bug.ts && echo "tslint success !"5.4.2
npm WARN saveError ENOENT: no such file or directory, open '/tmp/q/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN enoent ENOENT: no such file or directory, open '/tmp/q/package.json'
npm WARN q No description
npm WARN q No repository field.
npm WARN q No README data
npm WARN q No license field.

+ tslint@5.7.0
+ typescript@2.5.3
added 31 packages in 1.87s
/tmp/q
├─┬ tslint@5.7.0
│ ├─┬ babel-code-frame@6.26.0
│ │ ├─┬ chalk@1.1.3
│ │ │ ├── ansi-styles@2.2.1
│ │ │ ├── escape-string-regexp@1.0.5
│ │ │ ├─┬ has-ansi@2.0.0
│ │ │ │ └── ansi-regex@2.1.1
│ │ │ ├─┬ strip-ansi@3.0.1
│ │ │ │ └── ansi-regex@2.1.1 deduped
│ │ │ └── supports-color@2.0.0
│ │ ├── esutils@2.0.2
│ │ └── js-tokens@3.0.2
│ ├── colors@1.1.2
│ ├── commander@2.11.0
│ ├── diff@3.3.1
│ ├─┬ glob@7.1.2
│ │ ├── fs.realpath@1.0.0
│ │ ├─┬ inflight@1.0.6
│ │ │ ├── once@1.4.0 deduped
│ │ │ └── wrappy@1.0.2
│ │ ├── inherits@2.0.3
│ │ ├── minimatch@3.0.4 deduped
│ │ ├─┬ once@1.4.0
│ │ │ └── wrappy@1.0.2 deduped
│ │ └── path-is-absolute@1.0.1
│ ├─┬ minimatch@3.0.4
│ │ └─┬ brace-expansion@1.1.8
│ │   ├── balanced-match@1.0.0
│ │   └── concat-map@0.0.1
│ ├─┬ resolve@1.4.0
│ │ └── path-parse@1.0.5
│ ├── semver@5.4.1
│ ├── tslib@1.7.1
│ └─┬ tsutils@2.10.0
│   └── tslib@1.7.1 deduped
└── typescript@2.5.3

tslint success !
$ rm -rf package.json package-lock.json yarn.lock node_modules/ && yarn add typescript tslint && ./node_modules/.bin/tslint --config tslint.json bug.ts && echo "tslint success !"
yarn add v1.1.0
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 31 new dependencies.
├─ ansi-regex@2.1.1
├─ ansi-styles@2.2.1
├─ babel-code-frame@6.26.0
├─ balanced-match@1.0.0
├─ brace-expansion@1.1.8
├─ chalk@1.1.3
├─ colors@1.1.2
├─ commander@2.11.0
├─ concat-map@0.0.1
├─ diff@3.3.1
├─ escape-string-regexp@1.0.5
├─ esutils@2.0.2
├─ fs.realpath@1.0.0
├─ glob@7.1.2
├─ has-ansi@2.0.0
├─ inflight@1.0.6
├─ inherits@2.0.3
├─ js-tokens@3.0.2
├─ minimatch@3.0.4
├─ once@1.4.0
├─ path-is-absolute@1.0.1
├─ path-parse@1.0.5
├─ resolve@1.4.0
├─ semver@5.4.1
├─ strip-ansi@3.0.1
├─ supports-color@2.0.0
├─ tslib@1.7.1
├─ tslint@5.7.0
├─ tsutils@2.10.0
├─ typescript@2.5.3
└─ wrappy@1.0.2
Done in 2.06s.
tslint success !

@ajafff ajafff changed the title whitespace "check-separator" should allow jsdoc comments strange behavior when installing via yarn@1.1.0, e.g. whitespace "check-separator" in JSDoc Sep 29, 2017
@nikklassen
Copy link
Contributor

I was able to reproduce this issue with a slightly different file in my project.

class TsLintBug {
  public subtract(x: number, y: number): number;

  /**
   * Subtracts y from x.
   * @param foo bar
   * @returns {number}
   */
  public subtract(x: number, y: number): number {
    return x + y;
  }
}

One of the differences between yarn and npm is the version of tsutils that was installed. yarn 1.1.0 is currently installing 2.8.2, whereas npm installs 2.11.1 (both satisfy tslint's requirement of "^2.8.1"). When I upgraded to tsutils@2.11.1 I saw this false positive go away. Unfortunately I can't reproduce from a clean repo, only in my rather large project, but I hope this helps someone.

@PowerKiKi
Copy link

In my case I was also able to get rid of the error by upgrading my deps. I tried to isolate which one made a difference but was unable to. If somebody want to get to the bottom of this, i could provide before/after yarn.lock files...

@ajafff
Copy link
Contributor

ajafff commented Oct 3, 2017

If this is really related to the version of tsutils it should be fixed once #3258 lands.

That's still very strange. I didn't make any changes in tsutils that are related to those failures.
Maybe there's also a difference in the installed version of typescript?

@nikklassen
Copy link
Contributor

nikklassen commented Oct 3, 2017

I tried forcing my project's version of typescript, tsutils and tslint in a clean project, but still didn't observe the bug. When the bug occurred in my project, utils.forEachTokenWithTrivia was yielding an identifier token for foo (in @param foo), but when I upgraded tsutils it simply skipped that range. I can only guess that this occurs with a specific configuration of packages. I tried isolating them in my project's lockfile, but I have multiple versions of some of the dependencies tslint requires, so it doesn't seem likely I'll discover the magic combination. Based on what fixed my issue it would be reasonable to close this issue when tsutils is upgraded.

@ajafff
Copy link
Contributor

ajafff commented Oct 3, 2017

utils.forEachTokenWithTrivia was yielding an identifier token for foo (in @param foo)

I already thought that something like this would be the case. I'm puzzled why that could happen. The function excludes everything related to JSDoc.
It's only possible if something messes with the ts.SyntaxKind enum...

Anyways, thanks @nikklassen for digging into this. Let's hope this is fixed with the next release of tslint.

@Hotell
Copy link

Hotell commented Oct 4, 2017

related -> #3277

@fsz
Copy link

fsz commented Oct 4, 2017

In my project I could make the following observations:

  • tsutils not as direct dependency & typescript 2.4.2
    -> error
  • tsutils 2.11.2 as dependency & typescript 2.4.2
    -> no error
  • tsutils not as direct dependency & typescript >= 2.5.0
    -> no error

In all cases yarn gets tsutils 2.11.2.

BUT the error seems only to occur if I've got a dependency that again has a dependency on typescript. In my case this was compodoc.

@ajafff
Copy link
Contributor

ajafff commented Oct 4, 2017

Are there multiple different versions of typescript installed? I'd need to know which package uses which version of typescript.

@fsz
Copy link

fsz commented Oct 5, 2017

yarn list v1.1.0
├─ @compodoc/compodoc@1.0.1
│ └─ typescript@2.5.2
├─ @compodoc/ngd-core@2.0.0-alpha.3
│ └─ typescript@2.5.2
└─ typescript@2.4.2

But tsutils also installs its own typescript in version 2.5.2, though it is only a devDependency.

@ajafff
Copy link
Contributor

ajafff commented Oct 5, 2017

I was unable to create a minimal repro. I only got the faulty install on the first attempt. After that yarn had some even weirder bugs.
But someone else already reproduced the problem. See https://github.com/hexa00/repro-tslint-yarn
Here's the issue in the yarn issue tracker: yarnpkg/yarn#4539

@ajafff ajafff added the External label Oct 5, 2017
Ameobea added a commit to fat-whale/gdax-tt that referenced this issue Oct 9, 2017
 * Removed `"check-seperator"` from the `whitespace` rule to work around palantir/tslint#3251
 * Fixed float-typed object keys to use `[100.2]` instead of `"100.2"`
 * Project now lints without issue.
CjS77 pushed a commit to coinbase/coinbase-pro-trading-toolkit that referenced this issue Oct 9, 2017
* Start of implementation of BitMEX Market Feed

 * Created the `BitmexMarketFeed` class
   * Created interface definitions for messages received over the websocket
   * Based if off of the Gemini exchange feed class
   * Created methods for handling messages, initializing the websocket connection, and subscribing to symbols
 * TODO: Fix type issues and finish integrating the rest of the event emitters
 * TODO: Write tests
 * TODO: Update the sample file with the changes to the class

* Finish up handlers for orderbook updates

 * Properly map data from BitMEX's format into GTT's format
 * Remove left over logging statements
 * Handle one extra message type (welcome message)
 * Clean up some functions and code organization

* Started work on creating test for BitMEX Market Feed

 * Based off of the test for the Gemini Market Feed; Given simulated WS messages, checks to see that the state of the produced orderbook matches what's expected.

* Finished implementing tests

* Fixed tests by changing ID scheme

 * It turns out for non-level-3 data, GTT represents IDs by the stringified price level rather than the native order ID.  I changed the passed ID to reflect this.

* Fixed issues as per code review

 * Removed Ramnda dependency
 * Properly handle unknown and error messages in the BitMEX Market Feed
 * Add required `type` fields to the various message types

* Remove required `type` field on `UnknownMessage`

* Fixed lint issues

 * Removed `"check-seperator"` from the `whitespace` rule to work around palantir/tslint#3251
 * Fixed float-typed object keys to use `[100.2]` instead of `"100.2"`
 * Project now lints without issue.
@ajafff
Copy link
Contributor

ajafff commented Oct 14, 2017

Can someone on this thread verify that it works with yarn@1.2.1?

@nikklassen
Copy link
Contributor

I can confirm the issue I was seeing that was present with yarn@1.1.0 is no longer happening with yarn@1.2.1

@ajafff
Copy link
Contributor

ajafff commented Oct 14, 2017

Great. Let's close this.
If someone still has similar issues, please leave a comment or open a new issue.

seanf added a commit to zanata/zanata-platform that referenced this issue Jan 31, 2018
seanf added a commit to zanata/zanata-platform that referenced this issue Jan 31, 2018
seanf added a commit to zanata/zanata-platform that referenced this issue Feb 2, 2018
* Add TypeScript config
* Add extract-messages script
* Configure VS Code to use local TypeScript
* Remove Babel; add awesome-typescript-loader
* Allow at-loader to use cache, async error reporting
* Run tests with ts-jest
* Revert React imports (to fix PropTypes warning) and some other imports
* Use eval-source-map for dev in webpack
* Upgrade moment[-range]
* Disable ESLint line limits for JSX
* Disable ESLint for TS/TSX; add TSLint
* Add lots of @types
* Add TypeScript type comments to some JS files
* Convert some JS/JSX to TS/TSX
* Throw *new* Error for consistency
* Add tuple.ts to help unify proptypes enums with union types
* Use _ prefix for unused params
* Upgrade yarn to fix palantir/tslint#3251
* Add timestamp to watch builds
@davidmpaz
Copy link

davidmpaz commented Feb 2, 2018

Hi,

I am getting this issue now.

yarn -v       
1.3.2

and

"devDependencies": {
    "tslint": "^5.9.1",
    "tslint-eslint-rules": "^4.1.1",
    "typescript": "^2.6.1"
},

code where error shows:

/**
 * @param {FilterOptions} options   "here i got issue"
 * @return {PropertyDecorator}
 * @constructor
 */
export function Filter(options: FilterOptions): PropertyDecorator;

One thing to note is that is was happening only in the exported functions. Other private functions in module (not exported) did not have it.

Is there any deps I could force here. Error start to appear after I moved to yarn, with npm@5.6.0 was not a problem.

thanks in advance

David

@mauriedo
Copy link

This has just started to happen to us as well with yarn 1.3.2, in our case I am highly suspicious it might be related with the fact we have started to import a private package with tslint rules into the project. Downgrading to 1.0.1 fixes it, but not to 1.2.1.

@kelly-tock
Copy link

I upgraded to latest typescript today to 3.4, and this started happening to me as well. i'm on yarn 1.15.2

@kelly-tock
Copy link

ok, resolved it. I did yarn cache clean, and re-installed, and it is working now.

@tohjg
Copy link

tohjg commented Jun 10, 2019

This happen to me.

Tried to isolate my project from a monorepo (using lerna) and install via npm. Error still persist.
Both npm and yarn install tsutils 2.29.0.

import { FormGroup, ValidationErrors } from '@angular/forms';

/**
 * @example
    ^... missing whitespace
 * const formGroup = this.formBuilder.group({
 *  field1: '',
 *  field2: '',
 * }, {
 *  validators: [testValue]
 * })
 */
export function testValue(formGroup: FormGroup): ValidationErrors | null {
 ...
 return null;
}

Once I remove import statement, no error reported.

Following is my current setup.

yarn -v
1.16.0

package.json

{
  "devDependencies": {
    "tslint": "~5.16.0",
    "typescript": "~3.4.5"
  }
}

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

@palantir palantir locked and limited conversation to collaborators Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests