Repository: anz-bank/sysl
Pull request #939 (2020-07-08 05:47:56)

fixes attribute scope issues in type
Initially attributes in the higher scope replaces attributes that are in lower scope.
This shouldn't happen.

For example:
```sysl
App:
    !type attr:
        @a = 1
        i <: string:
            @a = 2
```
The resulted sysl module would replace the `a` attribute under the
field `i` with the value 1 which is higher in the scope.

This commit ensures that if attr values in lower scopes can not
be replaced by values in the higher scope.

Checklist:
- [x] Added related tests
- [ ] Made corresponding changes to the documentation


codecov[bot]: # [Codecov](https://codecov.io/gh/anz-bank/sysl/pull/939?src=pr&el=h1) Report
> Merging [#939](https://codecov.io/gh/anz-bank/sysl/pull/939?src=pr&el=desc) into [master](https://codecov.io/gh/anz-bank/sysl/commit/b37ce6254cec4a15899c576f3f02b5f367c9b1cd&el=desc) will **increase** coverage by `0.67%`.
> The diff coverage is `83.83%`.

```diff
@@            Coverage Diff             @@
##           master     #939      +/-   ##
==========================================
+ Coverage   83.35%   84.03%   +0.67%     
==========================================
  Files          73       74       +1     
  Lines       10609    10302     -307     
==========================================
- Hits         8843     8657     -186     
+ Misses       1432     1335      -97     
+ Partials      334      310      -24     
```


| [Impacted Files](https://codecov.io/gh/anz-bank/sysl/pull/939?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/pbutil/output.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL3BidXRpbC9vdXRwdXQuZ28=) | `76.00% <ø> (-0.48%)` | :arrow_down: |
| [pkg/parse/listener\_impl.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL3BhcnNlL2xpc3RlbmVyX2ltcGwuZ28=) | `89.50% <61.53%> (+<0.01%)` | :arrow_up: |
| [pkg/importer/importer.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL2ltcG9ydGVyLmdv) | `63.15% <63.15%> (ø)` | |
| [pkg/mod/gomod.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL21vZC9nb21vZC5nbw==) | `73.68% <66.66%> (+1.27%)` | :arrow_up: |
| [pkg/parse/parse.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL3BhcnNlL3BhcnNlLmdv) | `83.47% <69.56%> (-0.50%)` | :arrow_down: |
| [pkg/mod/module.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL21vZC9tb2R1bGUuZ28=) | `66.66% <72.72%> (+0.95%)` | :arrow_up: |
| [pkg/importer/formats.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL2Zvcm1hdHMuZ28=) | `77.27% <77.27%> (ø)` | |
| [pkg/importer/openapi.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL29wZW5hcGkuZ28=) | `81.73% <80.71%> (-7.72%)` | :arrow_down: |
| [pkg/importer/swagger.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL3N3YWdnZXIuZ28=) | `81.48% <88.88%> (+1.88%)` | :arrow_up: |
| [pkg/importer/xsd.go](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree#diff-cGtnL2ltcG9ydGVyL3hzZC5nbw==) | `91.01% <95.23%> (+0.64%)` | :arrow_up: |
| ... and [13 more](https://codecov.io/gh/anz-bank/sysl/pull/939/diff?src=pr&el=tree-more) | |


Patch:
diff --git a/pkg/parse/listener_impl.go b/pkg/parse/listener_impl.go
index a79baae8..92a321a7 100644
--- a/pkg/parse/listener_impl.go
+++ b/pkg/parse/listener_impl.go
@@ -765,7 +765,10 @@ func (s *TreeShapeListener) applyAnnotations(
 			if f.Attrs == nil {
 				f.Attrs = map[string]*sysl.Attribute{}
 			}
-			f.Attrs[varname] = attr
+			// if varname is already in lower scope, it shouldn't replace the value
+			if _, exists := f.Attrs[varname]; !exists {
+				f.Attrs[varname] = attr
+			}
 		}
 	}
 }


Test patch:
diff --git a/pkg/parse/Parser_test.go b/pkg/parse/Parser_test.go
index 931c6a55..f20010bf 100644
--- a/pkg/parse/Parser_test.go
+++ b/pkg/parse/Parser_test.go
@@ -589,6 +589,12 @@ func TestOpenAPI3(t *testing.T) {
 	testParseAgainstGoldenWithSourceContext(t, "tests/openapi3.sysl")
 }
 
+func TestAttrScope(t *testing.T) {
+	t.Parallel()
+
+	testParseAgainstGoldenWithSourceContext(t, "tests/attr_scope.sysl")
+}
+
 func TestUndefinedRootAbsoluteImport(t *testing.T) {
 	t.Parallel()
 
diff --git a/pkg/parse/tests/attr_scope.sysl b/pkg/parse/tests/attr_scope.sysl
new file mode 100644
index 00000000..8a0613dd
--- /dev/null
+++ b/pkg/parse/tests/attr_scope.sysl
@@ -0,0 +1,7 @@
+App:
+    !type scope:
+        @scope1 = "scope1"
+        x <: string:
+            @scope1 = "scope2"
+        y <: string:
+            @scope2 = "scope2"
diff --git a/pkg/parse/tests/attr_scope.sysl.golden.textpb b/pkg/parse/tests/attr_scope.sysl.golden.textpb
new file mode 100644
index 00000000..9c477184
--- /dev/null
+++ b/pkg/parse/tests/attr_scope.sysl.golden.textpb
@@ -0,0 +1,92 @@
+apps: {
+  key: "App"
+  value: {
+    name: {
+      part: "App"
+    }
+    types: {
+      key: "scope"
+      value: {
+        tuple: {
+          attr_defs: {
+            key: "x"
+            value: {
+              primitive: STRING
+              attrs: {
+                key: "scope1"
+                value: {
+                  s: "scope2"
+                }
+              }
+              source_context: {
+                file: "tests/attr_scope.sysl"
+                start: {
+                  line: 5
+                  col: 13
+                }
+                end: {
+                  line: 6
+                  col: 8
+                }
+              }
+            }
+          }
+          attr_defs: {
+            key: "y"
+            value: {
+              primitive: STRING
+              attrs: {
+                key: "scope1"
+                value: {
+                  s: "scope1"
+                }
+              }
+              attrs: {
+                key: "scope2"
+                value: {
+                  s: "scope2"
+                }
+              }
+              source_context: {
+                file: "tests/attr_scope.sysl"
+                start: {
+                  line: 7
+                  col: 13
+                }
+                end: {
+                  line: 8
+                }
+              }
+            }
+          }
+        }
+        attrs: {
+          key: "scope1"
+          value: {
+            s: "scope1"
+          }
+        }
+        source_context: {
+          file: "tests/attr_scope.sysl"
+          start: {
+            line: 3
+            col: 4
+          }
+          end: {
+            line: 8
+          }
+        }
+      }
+    }
+    source_context: {
+      file: "tests/attr_scope.sysl"
+      start: {
+        line: 1
+        col: 1
+      }
+      end: {
+        line: 1
+      }
+    }
+  }
+}
\ No newline at end of file