Added aliased import #258

kanghyojun · 2018-04-12T15:00:21Z

It's handy to avoid a name shadowing. It resolve #217.

However, it needs more test cases for merge.

kanghyojun · 2018-04-12T15:00:46Z

It is releated with #219

checkmate-bot · 2018-04-12T15:03:07Z

Checklist 🤔

`src/Nirum/Parser.hs`

If a new reserved keyword is introduced, it has to be also added to reservedKeywords set in the Nirum.Constructs.Identifier module.

codecov · 2018-04-13T02:10:43Z

Codecov Report

Merging #258 into master will increase coverage by 0.31%.
The diff coverage is 95.31%.

@@            Coverage Diff             @@
##           master     #258      +/-   ##
==========================================
+ Coverage   75.84%   76.16%   +0.31%     
==========================================
  Files          33       33              
  Lines        2414     2446      +32     
  Branches      132      132              
==========================================
+ Hits         1831     1863      +32     
  Misses        451      451              
  Partials      132      132

Impacted Files	Coverage Δ
src/Nirum/Targets/Python/Validators.hs	`83.13% <ø> (ø)`	⬆️
src/Nirum/Targets/Python/TypeExpression.hs	`93.1% <100%> (+0.12%)`	⬆️
src/Nirum/Parser.hs	`87.34% <100%> (+0.48%)`	⬆️
src/Nirum/TypeInstance/BoundModule.hs	`68.96% <100%> (+2.29%)`	⬆️
src/Nirum/Package/ModuleSet.hs	`93.61% <100%> (ø)`	⬆️
src/Nirum/Constructs/Module.hs	`70% <100%> (+4.09%)`	⬆️
src/Nirum/Constructs/Identifier.hs	`96.29% <100%> (+0.06%)`	⬆️
src/Nirum/Targets/Python/Serializers.hs	`79.48% <50%> (+2.56%)`	⬆️
src/Nirum/Constructs/TypeDeclaration.hs	`77.77% <90%> (-0.08%)`	⬇️

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 5630d29...6be588a. Read the comment docs.

dahlia · 2018-04-25T12:06:01Z

test/Nirum/Package/ModuleSetSpec.hs

-               , Import ["baz"] "qux" empty
-               ] Nothing
+      , Module
+            [ Import ["foo", "bar"] "xyz" "xyz" empty --MissingModulePathError


Please add a space right after --.

dahlia · 2018-04-25T12:14:50Z

src/Nirum/Parser.hs

-    return [Import path ident aSet | (ident, aSet) <- idents]
+    return [ Import path source imp aSet
+           | (imp, source, aSet) <- idents
+           ]


We should have tests for all combinations of the following conditions:

Empty parentheses Single import name w/o trailing comma Single import name w/ trailing comma Multiple import names w/o trailing comma Multiple import names w/ trailing comma

With as import foo () import foo (x as a) import foo (x as a,) import foo (x as a, y as b, c) import foo (x as a, y as b, c,)

Without as import foo() import foo (a) import foo(a, b, c,) import foo (a,) import foo(a, b, c)

We should test if the parser disallows and shows an error when there are duplicated alias names.

dahlia · 2018-04-25T15:14:41Z

test/Nirum/Constructs/ModuleSpec.hs

-                                    , (["zzz"], ["qqq", "ppp"])
+            imports mod1 `shouldBe` [ (["foo", "bar"], [ ("baz", "baz")
+                                                       , ("qux", "qux")
+                                                       ])


dahlia · 2018-04-25T15:14:49Z

test/Nirum/Constructs/ModuleSpec.hs

+                                    , (["xyz"], [("asdf", "asdf")])
+                                    , (["zzz"], [ ("qqq", "qqq")
+                                                , ("ppp", "ppp")
+                                                ])


dahlia · 2018-04-25T15:15:17Z

src/Nirum/TypeInstance/BoundModule.hs

+        Nothing -> toType
+                       coreModulePath
+                       identifier
+                       (DS.lookup identifier $ types coreModule)


dahlia · 2018-04-25T15:15:56Z

test/Nirum/Package/ModuleSetSpec.hs

-    , (["qux"], Module [ Import ["foo"] "abc" empty -- MissingImportError
-                       , Import ["foo"] "def" empty -- MissingImportError
+    , (["qux"], Module [ Import ["foo"] "abc" "abc" empty -- MissingImportError
+                       , Import ["foo"] "def" "def" empty -- MissingImportError
                       ] Nothing)


dahlia · 2018-04-25T15:16:30Z

test/Nirum/Package/ModuleSetSpec.hs

                       ] Nothing)
    ]

 circularImportsModules :: [(ModulePath, Module)]
 circularImportsModules =
-    [ (["asdf"], Module [ Import ["asdf"] "foo" empty
+    [ (["asdf"], Module [ Import ["asdf"] "foo" "foo" empty
                        , TypeDeclaration "bar" (Alias "text") empty
                        ] Nothing)


dahlia · 2018-04-25T15:16:38Z

test/Nirum/Package/ModuleSetSpec.hs

                        , TypeDeclaration "bar" (Alias "text") empty
                        ] Nothing)
-    , (["abc", "def"], Module [ Import ["abc", "ghi"] "bar" empty
+    , (["abc", "def"], Module [ Import ["abc", "ghi"] "bar" "bar" empty
                              , TypeDeclaration
                                    "foo" (Alias "text") empty
                              ] Nothing)


dahlia · 2018-04-25T15:16:43Z

test/Nirum/Package/ModuleSetSpec.hs

                              , TypeDeclaration
                                    "bar" (Alias "text") empty
                              ] Nothing)
-    , (["abc", "xyz"], Module [ Import ["abc", "def"] "foo" empty
+    , (["abc", "xyz"], Module [ Import ["abc", "def"] "foo" "foo" empty
                              , TypeDeclaration
                                    "baz" (Alias "text") empty
                              ] Nothing)


dahlia · 2018-04-25T15:17:43Z

test/Nirum/ParserSpec.hs

+        let (parse', expectError) = helperFuncs $ P.imports []
+        it "can single import name w/o trailing comma" $ do
+            parse' "import foo.bar (a);" `shouldBeRight`
+                [ Import ["foo", "bar"] "a" "a" empty ]


dahlia · 2018-04-25T15:18:07Z

test/Nirum/ParserSpec.hs

+            parse' "import foo.bar (a,);" `shouldBeRight`
+                [ Import ["foo", "bar"] "a" "a" empty ]
+            parse' "import foo.bar (a as qux,);" `shouldBeRight`
+                [ Import ["foo", "bar"] "qux" "a" empty ]


dahlia · 2018-04-25T15:19:13Z

src/Nirum/Parser.hs

-        <?> "names to import"
+    idents <- many' [] $ \ importNames' -> do
+        notFollowedBy $ choice [char ')', char ',' >> spaces >> char ')']
+        let forwardNames' = [ i | (i, _, _) <- importNames' ] ++


dahlia

Build is failing due to lint:

test/Nirum/TypeInstance/BoundModuleSpec.hs:56:1: too long line (83 chars)
test/Nirum/ParserSpec.hs:1180:1: too long line (81 chars)
test/Nirum/ParserSpec.hs:1185:1: too long line (81 chars)
test/Nirum/ParserSpec.hs:1206:1: too long line (82 chars)
test/Nirum/ParserSpec.hs:1215:1: too long line (83 chars)
test/Nirum/ParserSpec.hs:1255:1: too long line (91 chars)

kanghyojun force-pushed the import-aliasing branch from afd38bf to a69542e Compare April 13, 2018 01:36

kanghyojun force-pushed the import-aliasing branch from a69542e to 6b824da Compare April 19, 2018 13:40

kanghyojun requested a review from dahlia April 25, 2018 11:49

kanghyojun self-assigned this Apr 25, 2018

kanghyojun added typ:enhance Type: Enhancement/new feature cmp:compiler Component: Compiler backend (e.g., annotation processors, code generators) target:python dfct:hard Difficulty: Hard labels Apr 25, 2018

dahlia requested changes Apr 25, 2018

View reviewed changes

kanghyojun force-pushed the import-aliasing branch 3 times, most recently from 1e581a8 to 77b5274 Compare April 25, 2018 14:54

kanghyojun added 6 commits April 26, 2018 00:09

Parse import aliasing syntax

bad4d84

Add example for import aliasing

a0cb2c9

Change type signature of Import

3989b69

Apply changed Import

b084fd2

Write changelog

cd7b32a

Add reserved keyword "as"

2cdd1ca

kanghyojun force-pushed the import-aliasing branch from 77b5274 to 25da998 Compare April 25, 2018 15:09

dahlia requested changes Apr 25, 2018

View reviewed changes

kanghyojun added 4 commits April 26, 2018 00:26

Apply changed Import to ModuleSpec

60217b2

Apply changed Import to ModuleSetSpec

8ccf33a

Apply changed Import to CodeGenSpec

371391f

Apply changed Import to DocsSpec

fe98ee3

kanghyojun force-pushed the import-aliasing branch from 25da998 to 1a7da15 Compare April 25, 2018 15:26

dahlia requested changes Apr 25, 2018

View reviewed changes

Apply changed Import & Imported to BoundModuleSpec

a416bbb

kanghyojun added 5 commits April 30, 2018 20:46

Apply changed Parser.imports to ParserSpec

d31ecd0

Apply changed Import to TypeDeclarationSpec

61e24bf

Test added examples

9e33228

Parse trailing comma

ddd187b

Test aliasing import on Python

6be588a

kanghyojun force-pushed the import-aliasing branch from 1a7da15 to 6be588a Compare April 30, 2018 12:15

dahlia approved these changes Apr 30, 2018

View reviewed changes

kanghyojun merged commit 4fb4391 into nirum-lang:master Apr 30, 2018

dahlia added this to the Version 0.4.0 milestone May 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added aliased import #258

Added aliased import #258

kanghyojun commented Apr 12, 2018

kanghyojun commented Apr 12, 2018

checkmate-bot commented Apr 12, 2018 •

edited

Loading

codecov bot commented Apr 13, 2018 •

edited

Loading

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia Apr 25, 2018

dahlia left a comment

	Empty parentheses	Single import name w/o trailing comma	Single import name w/ trailing comma	Multiple import names w/o trailing comma	Multiple import names w/ trailing comma
With `as`	`import foo ()`	`import foo (x as a)`	`import foo (x as a,)`	`import foo (x as a, y as b, c)`	`import foo (x as a, y as b, c,)`
Without `as`	`import foo()`	`import foo (a)`	`import foo(a, b, c,)`	`import foo (a,)`	`import foo(a, b, c)`

Added aliased import #258

Added aliased import #258

Conversation

kanghyojun commented Apr 12, 2018

kanghyojun commented Apr 12, 2018

checkmate-bot commented Apr 12, 2018 • edited Loading

Checklist 🤔

src/Nirum/Parser.hs

codecov bot commented Apr 13, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dahlia left a comment

Choose a reason for hiding this comment

checkmate-bot commented Apr 12, 2018 •

edited

Loading

`src/Nirum/Parser.hs`

codecov bot commented Apr 13, 2018 •

edited

Loading