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

Add support for pedantic property access #40171

Merged
merged 7 commits into from Nov 2, 2020

Conversation

Kingwl
Copy link
Contributor

@Kingwl Kingwl commented Aug 21, 2020

Fixes #40170

@Kingwl Kingwl marked this pull request as draft August 21, 2020 09:34
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 21, 2020
@Kingwl Kingwl marked this pull request as ready for review August 24, 2020 03:15
@sandersn sandersn added this to Not started in PR Backlog Sep 1, 2020
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions for the wording of messages.

src/compiler/diagnosticMessages.json Outdated Show resolved Hide resolved
src/compiler/diagnosticMessages.json Outdated Show resolved Hide resolved
src/compiler/diagnosticMessages.json Outdated Show resolved Hide resolved
PR Backlog automation moved this from Not started to Waiting on author Sep 8, 2020
@sandersn sandersn self-assigned this Sep 8, 2020
@sandersn
Copy link
Member

sandersn commented Sep 8, 2020

Also, at the Friday (Sep 4) design meeting, we decided not to create a single naming scheme for pedantic options, but (1) to name them for what they do (2) use the "no-" prefix only if it makes sense, since we already have some pedantic options named that, like noUnusedLocals, noFallthroughCasesInSwitch, etc.

I would suggest noPropertyAccessFromIndexSignature.

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good, @sandersn’s messaging suggestions look good

@ssledorze
Copy link

Really useful and awaited! :)
Looking forward to that PR being merged..
Is there some information on the expected release it would be part of?

@Kingwl
Copy link
Contributor Author

Kingwl commented Sep 25, 2020

@ssledorze After 4.1 release, I guess.

PR Backlog automation moved this from Waiting on author to Needs merge Oct 28, 2020
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this after 4.1 RC.

@ssledorze
Copy link

I'll merge this after 4.1 RC.

should I understand it will be in the 4.1 release @sandersn ?

@sandersn
Copy link
Member

No, we're finishing up 4.1 RC right now, then 4.1 will get its own branch. After that, commits to master will go into 4.2. So this will be in 4.2.

@Kingwl
Copy link
Contributor Author

Kingwl commented Oct 29, 2020

@typescript-bot pack this.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 29, 2020

Heya @Kingwl, I've started to run the tarball bundle task on this PR at 9fad505. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 29, 2020

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/88073/artifacts?artifactName=tgz&fileId=1062C274A86E3AACC8BC8EC93C079C87B60E684E9F8015BD9366FD3BC705D07F02&fileName=/typescript-4.1.0-insiders.20201029.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #40170. This makes it less likely that we'll review or accept this PR. Try to get the originating issue accepted.

@sandersn sandersn merged commit ce8d702 into microsoft:master Nov 2, 2020
PR Backlog automation moved this from Needs merge to Done Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
PR Backlog
  
Done
Development

Successfully merging this pull request may close these issues.

pedanticPropertyLookup
5 participants