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

Linting with absolute path to tmp file is broken in macOS Catalina #4880

Closed
dhleong opened this issue Oct 20, 2019 · 7 comments · Fixed by #4887
Closed

Linting with absolute path to tmp file is broken in macOS Catalina #4880

dhleong opened this issue Oct 20, 2019 · 7 comments · Fixed by #4887

Comments

@dhleong
Copy link

dhleong commented Oct 20, 2019

Bug Report

  • TSLint version: 5.20.0
  • TypeScript version: 3.6.4
  • Running TSLint via: (pick one) CLI

Reproduction using TSLint Playground

N/A

TypeScript code being linted

// ...
import { DiscoveryId, IDiscoveredChange, IDiscovery } from "./base";
// ...

with tslint.json configuration:

    "defaultSeverity": "error",
    "extends": [
        "tslint:recommended"
    ],
    "jsRules": {},
    "rules": {
        "arrow-parens": [true, "ban-single-arg-parens"],
        "curly": [true, "ignore-same-line"],
        "max-classes-per-file": false
    },
    "rulesDirectory": []
}

via CLI invocation:

tslint -c ~/git/shougun/tslint.json --fix /var/folders/9q/s674cplx7qv122xbwcchw3jm0000gn/T/vEeQL2Q/239/composite.ts

Actual behavior

{ Error: ENOENT: no such file or directory, open '../../../../var/folders/9q/s674cplx7qv122xbwcchw3jm0000gn/T/vEeQL2Q/223/composite.ts'
    at Object.fs.openSync (fs.js:559:3)
    at Object.fs.writeFileSync (fs.js:1289:33)
    at /Users/dhleong/.npm-packages/lib/node_modules/tslint/lib/linter.js:198:16
    at Map.forEach (<anonymous>)
    at Linter.applyFixes (/Users/dhleong/.npm-packages/lib/node_modules/tslint/lib/linter.js:186:21)
    at _loop_1 (/Users/dhleong/.npm-packages/lib/node_modules/tslint/lib/linter.js:169:33)
    at Linter.applyAllFixes (/Users/dhleong/.npm-packages/lib/node_modules/tslint/lib/linter.js:176:13)
    at Linter.lint (/Users/dhleong/.npm-packages/lib/node_modules/tslint/lib/linter.js:116:33)
    at /Users/dhleong/.npm-packages/lib/node_modules/tslint/lib/runner.js:183:32
    at step (/Users/dhleong/.npm-packages/lib/node_modules/tslint/node_modules/tslib/tslib.js:136:27)
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '../../../../var/folders/9q/s674cplx7qv122xbwcchw3jm0000gn/T/vEeQL2Q/223/composite.ts' }

Expected behavior

Fix the import ordering

Notes

Not sure if this is the appropriate fix, but using path.resolve(filePath) instead of the raw filePath in the call to writeFileSync of Linter.applyFixes seems to fix it. May be related to Catalina's changes to the root directory. It does resolve that weird relative path (note that the input to the CLI was not relative).

Happy to provide a PR with that one-line change if you like.

@JoshuaKGoldberg
Copy link
Contributor

Wow, this Catalina update is really getting a bad rep... 😅

May be related to Catalina's changes to the root directory.

Could you please post more info on that @dhleong? I'd be happy to look deeper into / review a PR for path.resolve(filePath) on Windows and Mac with a better understanding of what's going wrong.

Thanks for filing!

@dhleong
Copy link
Author

dhleong commented Oct 26, 2019

Yeah... I was too excited about iTunes not popping up when I plugged in iDevices and decided not to wait for the .1; big mistake.

Anyway I'm not an expert, but here's an article about the change. Basically / is a read-only file system on Catalina. The user directories, etc. are mounted in some special way so they can be written to as normal, as I understand it. I'm just guessing that it's related, but the tmp files that ALE creates are in /var/private/... so perhaps something about that is tripping up node's path stuff? I haven't looked too deeply into what tslint is actually doing with the path here, but it does seem weird that it's using that long relative path instead of the absolute path as passed in.

Thanks for taking a look!

@JoshuaKGoldberg
Copy link
Contributor

FYI @adidahiya - there might be some historical context? I can test out a PR that comes in on Mac Mojave and Windows 10.

@i1skn
Copy link
Contributor

i1skn commented Nov 5, 2019

@dhleong I've encountered the same issue and have created a PR to fix this as you've suggested. Hopefully it's with you, otherwise fill free to create your own and I would close my one. Thanks!

@dhleong
Copy link
Author

dhleong commented Nov 5, 2019

Hey thanks @i1skn ! I've been sidetracked with work and that looks about like what I would have submitted so feel free to run with it, I don't mind :)

@JoshuaKGoldberg
Copy link
Contributor

The fix in #4887 will be available in TSLint v6. Thanks!

@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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants