roblox-ts / eslint-plugin-roblox-ts

7 stars 6 forks source link

The rule roblox-ts/lua-truthiness can cause some awkward auto-fixing in the linter which produces logical errors #23

Closed iamacup closed 7 months ago

iamacup commented 3 years ago

Version

1.0.0-beta.14

"devDependencies": {
    "@rbxts/compiler-types": "^1.0.0-beta.14.0",
    "@rbxts/types": "^1.0.437",
    "@typescript-eslint/eslint-plugin": "^4.13.0",
    "@typescript-eslint/parser": "^4.13.0",
    "eslint": "^7.17.0",
    "eslint-config-prettier": "^7.1.0",
    "eslint-plugin-prettier": "^3.3.1",
    "eslint-plugin-roblox-ts": "0.0.24",
    "prettier": "^2.2.1",
    "typescript": "^4.1.3"
  },
  "dependencies": {
    "@rbxts/net": "^2.0.0-alpha.6",
    "@rbxts/networked-signals": "0.0.4",
    "@rbxts/services": "^1.1.2",
    "@rbxts/t": "^2.1.1"
  }

Hey

This is potentially a non-issue so I will leave it up to you guys to decide, it’s more of an observation with a suggestion.

The rule roblox-ts/lua-truthiness can cause some awkward auto-fixing in the linter which produces logical errors that, if not caught when the linter runs, could create annoying to debug problems.

Consider this code:

type PlayerRecord = {
    name: string;
    coins: number;
};

type PlayerRecords = {
    [key: string]: PlayerRecord | undefined;
};

const players: PlayerRecords = {};

const playerTest: PlayerRecord = {
    name: "testname",
    coins: 10,
};

players["testname"] = playerTest;

If you put this into your editor:

const coins1 = players["testname"]?.coins || 0; 
print(coins1); // 10

const coins2 = players["testnameDOESNOTEXIST"]?.coins || 0; 
print(coins2); // 0

And the linter runs as it is setup by default - it will auto-fix to this:

const coins1 = players["testname"]?.coins !== undefined || 0; 
print(coins1); // true

const coins2 = players["testnameDOESNOTEXIST"]?.coins !== undefined || 0; 
print(coins2); // 0

which obviously has a different output than what the programmer is most likely expecting when they wrote the line.

I don’t know if it’s possible to fix this case within the rule itself?

But if not, I think it would be preferable for roblox-ts/lua-truthiness to not auto-fix by default, if that is a thing that can be configured?

Obviously, the fix for this on a case by case basis is to add eslint-disable-next-line roblox-ts/lua-truthiness to lines where this happens.

Again, feel free to close this as it’s not really a big deal, it really only applies if people write code and the linter changes it without them noticing / realising.

Thanks, and roblox-ts is truly awesome :)

iamacup commented 3 years ago

https://github.com/roblox-ts/roblox-ts/issues/1229

Dionysusnu commented 7 months ago

For reference, this auto-fix eventually got disabled in f43ef43