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

Fix several issues with object type reference #18404

Merged
merged 6 commits into from
Sep 2, 2019

Conversation

MaryamZi
Copy link
Member

@MaryamZi MaryamZi commented Sep 1, 2019

Purpose

$title.

Fixes #17920 Referencing objects with fields/methods with module level visibility is not restricted across modules
Fixes #18370 No validation for function param visibility match when referencing objects
Fixes #13822 Referencing two abstract objects which both have a method with the same signature does not cause a compilation error
Fixes #18402 Object type reference attempts copying methods of invalid references

Approach

Describe how you are implementing the solutions along with the design details.

Samples

Provide high-level details about the samples related to this feature.

Remarks

List any other known issues, related PRs, TODO items, or any other notes related to the PR.

Check List

  • Read the Contributing Guide
  • Required Balo version update
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

@MaryamZi MaryamZi added the Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. label Sep 1, 2019
@codecov-io
Copy link

Codecov Report

Merging #18404 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #18404   +/-   ##
=======================================
  Coverage   14.82%   14.82%           
=======================================
  Files          48       48           
  Lines        1268     1268           
  Branches      201      201           
=======================================
  Hits          188      188           
  Misses       1066     1066           
  Partials       14       14

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea3ca59...ccf5e3d. Read the comment docs.

@@ -1688,6 +1690,7 @@ private void createDummyTypeDefSymbol(TypeDefinition typeDefNode, SymbolEnv env)

private void resolveReferencedFields(BLangStructureTypeNode structureTypeNode, SymbolEnv typeDefEnv) {
List<BSymbol> referencedTypes = new ArrayList<>();
List<BLangType> invalidTypeRefs = new ArrayList<>();
// Get the inherited fields from the type references
structureTypeNode.referencedFields = structureTypeNode.typeRefs.stream().flatMap(typeRef -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, This is not a good stream implementation. we should refactor this later.

}

BObjectType objectType = (BObjectType) referredType;
if (structureTypeNode.type.tsymbol.owner != referredType.tsymbol.owner) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There can be scenario, this logic fails, Like, local type def. Better to validate pkg, instated.

@hasithaa hasithaa merged commit 21f9a53 into ballerina-platform:master Sep 2, 2019
@MaryamZi MaryamZi deleted the obj-reference-fixes branch June 15, 2020 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times.
Projects
None yet
3 participants