Skip to content

Double error on invalid delete of readonly property #53082

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

Closed
jakebailey opened this issue Mar 3, 2023 · 16 comments Β· Fixed by #55449
Closed

Double error on invalid delete of readonly property #53082

jakebailey opened this issue Mar 3, 2023 · 16 comments Β· Fixed by #55449
Labels
Bug A bug in TypeScript Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Milestone

Comments

@jakebailey
Copy link
Member

Bug Report

πŸ”Ž Search Terms

operand delete operator double diagnostic error

πŸ•— Version & Regression Information

  • This changed between versions 3.9 and 4.0

Before 4.0, we only gave one error here. In 4.0 and above, we give two errors.

⏯ Playground Link

Playground Link

πŸ’» Code

class Foo {}
delete Foo.name;

πŸ™ Actual behavior

Two errors

πŸ™‚ Expected behavior

One error

@jakebailey jakebailey changed the title Double error on in valid delete of readonly property Double error on invalid delete of readonly property Mar 3, 2023
@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this Good First Issue Well scoped, documented and has the green light labels Mar 7, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 7, 2023
@ahmad-04
Copy link

ahmad-04 commented Mar 7, 2023

Hi, I wanted to work on this where should I start.

@Bismay5467
Copy link

Hey ! I have possible explanation for the second error.

Error : The operand of a 'delete' operator must be optional.

Explanation : That was a false positive where the prototype chain still includes the property - bcoz the delete statement only works on direct properties, and not on inherited properties. This got corrected in the later versions of typescript.

One possible fix : declare 'name' member inside class as optional.

@jakebailey
Copy link
Member Author

This bug doesn't seem to reproduce in a compiler or fourslash test, so this looks to be something in the services causing a duplicate error to occur (not uncommon). In particular, the The operand of a 'delete' operator cannot be a read-only property. message does not show up in testing.

@ahmad2901 Likely, this needs a test like in #52696 or #52373 which baselines the old bad behavior, followed by a change which fixes the bug and changes the baseline to only contain one error.

@Bismay5467 I don't think that's related, given it doesn't reproduce in a compiler test, only in the editor, and this bug is about that inconsistency.

@ahmad-04
Copy link

Hey, @jakebailey I am new to typescript can you recommend where should I get started?

@jakebailey
Copy link
Member Author

The place to start would be to find all places in the checker which emit this error message, set a breakpoint at each, and figure out in which conditions both appear, and see which one shouldn't be happening.

@ahmad-04
Copy link

sure thanks for your help.

@ahmad-04
Copy link

@jakebailey by checker do you mean tests folder??

@somebody1234
Copy link

somebody1234 commented Mar 18, 2023

@jakebailey i can't repro on 5.0.2 and 4.0 - i get the second error even when running tsc, unless that's not what compiler and fourslash tests do? note that checkDeleteExpressionMustBeOptional explicitly checks for strictNullChecks so it will not be emitted if that's off

the fix looks like it'd be simple to me so i'm likely just misunderstanding something here

            if (isReadonlySymbol(symbol)) {
                error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_read_only_property);
                // missing return, so tsc *may* emit a second error from the below function call
            }
            checkDeleteExpressionMustBeOptional(expr, symbol);

@shoeb00
Copy link

shoeb00 commented Apr 18, 2023

Hey @jakebailey , any updates on this issue? I am unable to replicate this issue. I have tried versions 3.0, 3.9, 4.0 and 4.9 and there was a single error for all of them

test.ts:2:8 - error TS2704: The operand of a 'delete' operator cannot be a read-only property.

2 delete Foo.name;
~~~~~~~~

Found 1 error in test.ts:2

@jakebailey
Copy link
Member Author

I don't have any updates. This only reproduces in the language service and the playground, as I said before. If you check the latter by the link in my issue, you'll see it says there are two errors on the right.

@shoeb00
Copy link

shoeb00 commented Apr 18, 2023

This is my first time contributing to an open-source project, I would really appreciate your help. I have cloned the repo and switched to the release-4.0 branch then I ran these commands

npm install && npm run build

Then I created a test.ts file with the code mentioned in the issue then to run that file I used this command

node built/local/tsc.js test.ts

Output:

test.ts:2:8 - error TS2704: The operand of a 'delete' operator cannot be a read-only property.

2 delete Foo.name;


Found 1 error in test.ts:2

I am actively looking for issues where I can contribute, please feel free to recommend any other issues/bugs or study material that can help me

@gooddavvy
Copy link

What are the errors? Can you be more specific?

@somebody1234
Copy link

@gooddavvy check the "Playground Link" heading in the original post

@onsevenine
Copy link

I think to it's trying to get an operand which could be empty, a property which has no use but still exist in the object.

@SauravChittal
Copy link

Is this issue fixed, or could I work on it?

@jakebailey
Copy link
Member Author

There is #53961 which presumably fixes the issue but isn't yet complete.

Also note: https://github.com/microsoft/TypeScript/blob/main/CONTRIBUTING.md#issue-claiming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Good First Issue Well scoped, documented and has the green light Help Wanted You can do this
Projects
None yet
9 participants