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

Rework deprecated functionality #159

Merged
merged 11 commits into from
Nov 22, 2022
Merged

Rework deprecated functionality #159

merged 11 commits into from
Nov 22, 2022

Conversation

blitz-1306
Copy link
Contributor

@blitz-1306 blitz-1306 commented Oct 18, 2022

Preface

Previously we introduced new InferType component in #142 and #152 PRs, also in 10.3.0 release. We also realized that it would impact on backward compatibility due to older type inference functions would significantly change their interface. We postponed this step for a while to inform dependent packages about further changes. This PR reworks a lot of deprectaed functions and adapts them to a newer type inference approach.

Changes

  • Rework deprecated functionality to fully support newer type inference component.
  • Use typeString parser with ModuleType and InaccessibleDynamicType for tests only (remove from common codebase).
  • Updated dependencies.
  • Fix affected tests.

Notes

Regards.

@blitz-1306 blitz-1306 added enhancement New feature or request breaking change Changes that would cause a backward compatibility break labels Oct 18, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2022

Codecov Report

Merging #159 (8d7bb0a) into master (c4feb1a) will increase coverage by 0.35%.
The diff coverage is 86.01%.

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
+ Coverage   91.03%   91.39%   +0.35%     
==========================================
  Files         269      266       -3     
  Lines        6294     6227      -67     
  Branches     1229     1214      -15     
==========================================
- Hits         5730     5691      -39     
+ Misses        314      290      -24     
+ Partials      250      246       -4     
Impacted Files Coverage Δ
.../implementation/declaration/contract_definition.ts 100.00% <ø> (ø)
...ast/implementation/declaration/error_definition.ts 100.00% <ø> (ø)
...ast/implementation/declaration/event_definition.ts 100.00% <ø> (+5.88%) ⬆️
.../implementation/declaration/function_definition.ts 100.00% <ø> (ø)
.../implementation/declaration/modifier_definition.ts 100.00% <ø> (+5.26%) ⬆️
...implementation/declaration/variable_declaration.ts 100.00% <ø> (ø)
src/ast/implementation/expression/function_call.ts 100.00% <ø> (+21.21%) ⬆️
src/types/ast/index.ts 100.00% <ø> (ø)
src/types/ast/internal/index.ts 100.00% <ø> (ø)
src/types/builtins.ts 100.00% <ø> (ø)
... and 14 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@blitz-1306 blitz-1306 changed the title Rework deprectated functionality Rework deprecated functionality Oct 18, 2022
@blitz-1306 blitz-1306 marked this pull request as ready for review October 19, 2022 08:19
@blitz-1306 blitz-1306 requested a review from cd1m0 October 19, 2022 08:19
@blitz-1306
Copy link
Contributor Author

I think we should not merge until we stabilize Scribble behavior, while it is using this branch. See failures in Consensys/scribble#200.

Copy link
Contributor

@cd1m0 cd1m0 left a comment

Choose a reason for hiding this comment

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

1 note and 1 question.

src/ast/definitions.ts Show resolved Hide resolved
src/types/builtins.ts Show resolved Hide resolved
src/types/utils.ts Show resolved Hide resolved
Copy link
Contributor

@cd1m0 cd1m0 left a comment

Choose a reason for hiding this comment

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

One last question, sorry I forgot one file last review

src/types/infer.ts Show resolved Hide resolved
src/types/infer.ts Show resolved Hide resolved
Copy link
Contributor

@cd1m0 cd1m0 left a comment

Choose a reason for hiding this comment

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

lgtm. Some issues for follow up work.

@blitz-1306 blitz-1306 merged commit 0acc69f into master Nov 22, 2022
@blitz-1306 blitz-1306 deleted the rework-deprecated-funcs branch November 22, 2022 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes that would cause a backward compatibility break enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants