[Python] Import fixes (#4035)

* [Python] Fix #3965: importSideEffects shouldn't generate identifier

* [Python] Fix #3481: Resolve also paths for non-qualified imports

* [Python] Ignore backwards paths in imports & tests

* Update changelog

---------

Co-authored-by: Maxime Mangel <[email protected]>

Author: alfonsogarciacaro

src/Fable.Cli/CHANGELOG.md

@@ -9,14 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-* [Python] - Print root module and module function comments (by @alfonsogarciacaro)
+* [Python] Print root module and module function comments (by @alfonsogarciacaro)
 * [Rust] Add support for module comments (by @ncave)
 * [Rust] Add support for null strings (by @ncave)
 
 ### Fixed
 
 * [JS/TS] - Fix anonymous record printing (#4029) (by @alfonsogarciacaro)
 * [Python] - Fix #3998: PhysicalEquality (by @alfonsogarciacaro)
+* [Python] Resolve relative paths for non-qualified imports (#3481) (by @alfonsogarciacaro)
+* [Python] `importSideEffects` shouldn't generate identifier (#3965) (by @alfonsogarciacaro)
 * [JS/TS] Fix #4031: Hoist vars locally in for and while loops (@alfonsogarciacaro)
 
 ## 5.0.0-alpha.9 - 2025-01-28

src/Fable.Cli/Pipeline.fs

@@ -252,10 +252,8 @@ module Python =
                         |> Array.choose (fun part ->
                             i <- i + 1
 
-                            if part = "." then
+                            if part = "." || part = ".." then
                                 None
-                            elif part = ".." then
-                                Some ""
                             elif i = parts.Length - 1 then
                                 Some(normalizeFileName part)
                             else

src/Fable.Compiler/CHANGELOG.md

@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * [JS/TS] - Fix anonymous record printing (#4029) (by @alfonsogarciacaro)
 * [Python] - Fix #3998: PhysicalEquality (by @alfonsogarciacaro)
+* [Python] Resolve relative paths for non-qualified imports (#3481) (by @alfonsogarciacaro)
+* [Python] `importSideEffects` shouldn't generate identifier (#3965) (by @alfonsogarciacaro)
 * [JS/TS] Fix #4031: Hoist vars locally in for and while loops (@alfonsogarciacaro)
 
 ## 5.0.0-alpha.9 - 2025-01-28

src/Fable.Transforms/Python/Fable2Python.fs

@@ -21,7 +21,7 @@ type ReturnStrategy =
 type Import =
     {
         Module: string
-        LocalIdent: Identifier option
+        LocalIdent: Identifier
         Name: string option
     }
 
@@ -4249,17 +4249,17 @@ module Util =
                 match im.Name with
                 | Some "*"
                 | Some "default" ->
-                    let (Identifier local) = im.LocalIdent.Value
+                    let (Identifier local) = im.LocalIdent
 
                     if moduleName <> local then
-                        Some moduleName, Alias.alias im.LocalIdent.Value
+                        Some moduleName, Alias.alias im.LocalIdent
                     else
-                        None, Alias.alias im.LocalIdent.Value
+                        None, Alias.alias im.LocalIdent
                 | Some name ->
                     let name = Naming.toSnakeCase name
 
-                    Some moduleName, Alias.alias (Identifier(Helpers.clean name), ?asname = im.LocalIdent)
-                | None -> None, Alias.alias (Identifier(moduleName), ?asname = im.LocalIdent)
+                    Some moduleName, Alias.alias (Identifier(Helpers.clean name), asname = im.LocalIdent)
+                | None -> None, Alias.alias (Identifier(moduleName), asname = im.LocalIdent)
             )
             |> List.groupBy fst
             |> List.map (fun (a, b) -> a, List.map snd b)
@@ -4290,16 +4290,9 @@ module Util =
     let getIdentForImport (ctx: Context) (moduleName: string) (name: string option) =
         // printfn "getIdentForImport: %A" (moduleName, name)
         match name with
-        | None -> Path.GetFileNameWithoutExtension(moduleName) |> Identifier |> Some
-        | Some name ->
-            match name with
-            | "default"
-            | "*" -> Path.GetFileNameWithoutExtension(moduleName)
-            | _ -> name
-            |> Naming.toSnakeCase
-            |> getUniqueNameInRootScope ctx
-            |> Identifier
-            |> Some
+        | None -> Path.GetFileNameWithoutExtension(moduleName)
+        | Some name -> name |> Naming.toSnakeCase |> getUniqueNameInRootScope ctx
+        |> Identifier
 
 module Compiler =
     open Util
@@ -4317,47 +4310,59 @@ module Compiler =
 
             member _.GetImportExpr(ctx, moduleName, ?name, ?r) =
                 // printfn "GetImportExpr: %A" (moduleName, name)
-                let cachedName = moduleName + "::" + defaultArg name "module"
+                let name =
+                    match name with
+                    | None
+                    | Some null -> ""
+                    | Some name -> name.Trim()
+
+                let isQualifiedPythonImport =
+                    match name with
+                    | ""
+                    | "default"
+                    | "*" -> false
+                    | _ -> true
+
+                let cachedName =
+                    moduleName
+                    + "::"
+                    + if isQualifiedPythonImport then
+                          name
+                      else
+                          ""
 
                 match imports.TryGetValue(cachedName) with
-                | true, i ->
-                    match i.LocalIdent with
-                    | Some localIdent -> Expression.identifier localIdent
-                    | None -> Expression.none
+                | true, i -> i.LocalIdent |> Expression.identifier
                 | false, _ ->
-                    let local_id = getIdentForImport ctx moduleName name
-
-                    match name with
-                    | Some "*"
-                    | None ->
-                        let i =
-                            {
-                                Name = None
-                                Module = moduleName
-                                LocalIdent = local_id
-                            }
-
-                        imports.Add(cachedName, i)
-                    | Some name ->
-                        let i =
-                            {
-                                Name =
-                                    if name = Naming.placeholder then
-                                        "`importMember` must be assigned to a variable" |> addError com [] r
-
-                                        name
-                                    else
-                                        name
-                                    |> Some
-                                Module = moduleName
-                                LocalIdent = local_id
-                            }
-
-                        imports.Add(cachedName, i)
-
-                    match local_id with
-                    | Some localId -> Expression.identifier localId
-                    | None -> Expression.none
+                    let local_id =
+                        if isQualifiedPythonImport then
+                            Some name
+                        else
+                            None
+                        |> getIdentForImport ctx moduleName
+
+                    let importName =
+                        match name with
+                        | _ when not isQualifiedPythonImport -> None
+                        | Naming.placeholder ->
+                            "`importMember` must be assigned to a variable" |> addError com [] r
+                            Some name
+                        | _ -> Some name
+
+                    let i =
+                        {
+                            Name = importName
+                            Module = moduleName
+                            LocalIdent = local_id
+                        }
+
+                    imports.Add(cachedName, i)
+
+                    // If the import member is empty we understand this is done for side-effects only
+                    if name = "" then
+                        Expression.none
+                    else
+                        Expression.identifier local_id
 
             member _.GetAllImports() =
                 imports.Values :> Import seq |> List.ofSeq

src/Fable.Transforms/Python/PythonPrinter.fs

@@ -873,6 +873,12 @@ let run writer (program: Module) : Async<unit> =
             | ImportFrom({ Module = Some(Identifier path) } as info) ->
                 let path = printer.MakeImportPath(path)
                 ImportFrom { info with Module = Some(Identifier path) }
+            | Import({ Names = names }) ->
+                let names =
+                    names
+                    |> List.map (fun n -> { n with Name = printer.MakeImportPath(n.Name.Name) |> Identifier })
+
+                Import { Names = names }
             | decl -> decl
             |> printDeclWithExtraLine false printer
 

tests/Python/TestPyInterop.fs

@@ -164,4 +164,44 @@ let factorial (count : int) : int =
 let ``test emitPyStatement works with parameters`` () =
     factorial 5 |> equal 120
 
+[<Fact>]
+let ``test importSideEffects`` () = // See #3965
+    importSideEffects "./native_code.py"
+    let mutable x = 3
+    Py.python $"""
+    {x} = native_code.add5({x} + 2)
+    """
+    x |> equal 10
+
+type NativeCode =
+    abstract add5: int -> int
+
+[<Fact>]
+let ``test importAll`` () =
+    let nativeCode: NativeCode = importAll "./native_code.py"
+    3 |> nativeCode.add5 |> equal 8
+
+let add5 (x: int): int = importMember "./native_code.py"
+
+[<Fact>]
+let ``test importMember`` () =
+    add5 -1 |> equal 4
+
+    // Cannot use the same name as Fable will mangle the identifier
+    let add7: int -> int = importMember "./native_code.py"
+    add7 12 |> equal 19
+
+    let add5': int -> int = import "add5" "./native_code.py"
+    add5' 12 |> equal 17
+
+    let multiply3 (x: int): int = importMember "./more_native_code.py"
+    multiply3 9 |> equal 27
+
+[<ImportAll("./native_code.py")>]
+let nativeCode: NativeCode = nativeOnly
+
+[<Fact>]
+let ``test ImportAll works with relative paths`` () = // See #3481
+    3 |> nativeCode.add5 |> equal 8
+
 #endif

tests/Python/more_native_code.py

@@ -0,0 +1,2 @@
+def multiply3(x: int) -> int:
+    return x * 3

tests/Python/native_code.py

@@ -0,0 +1,6 @@
+def add5(x: int) -> int:
+    return x + 5
+
+
+def add7(x: int) -> int:
+    return x + 7