[gbc] Handle forward- and backslashes in paths

Since .bond files can be authored on platforms with different directory separator characters, the paths in build scripts and Bond `import` statements may use a forwardslash when a backslash is expected or vice versa. In order to handle this, gbc now normalizes anything that is a path (file paths, response file items, import directories, import statement paths, output directories, &c.) to use the platform's preferred directory separator character. Tests for C++, C#, and Java have been updated to use some imports with mixed slashes. gbc doesn't use a dedicate type for file paths. `FilePath` is just a type synonym for `String`, so some places may have been inadvertently missed. This does now mean that gbc cannot process files, on, say, Linux, with backslashes in their names. This is expected to be rare, and is deemed an acceptable compromise for a cross-platform tool. Fixes microsoft#869
chwarr · May 10, 2018 · 74687e1 · 74687e1
1 parent d08de0d
commit 74687e1
Show file tree

Hide file tree

Showing 45 changed files with 1,199 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -44,6 +44,9 @@ different versioning scheme, following the Haskell community's
 * C++ codegen now generates an `allocator_type` typedef for a struct when the
   `--allocator` option is passed to `gbc`, instead of specializing `std::uses_allocator`.
 * `import` statements can now end with an optional semicolon.
+* File and directory paths on the command line, in response files, or in
+  `import` statements can now use a mix of forward and backslashes. [Issue
+  #869](https://github.com/Microsoft/bond/issues/869)
 
 ### C++ ###
 
@@ -130,7 +133,7 @@ different versioning scheme, following the Haskell community's
   constructors with parameters for each field. This functionality will
   change in the future and may be removed. [Pull request
   #857](https://github.com/Microsoft/bond/pull/857)
-	
+
 
 ## 7.0.2: 2017-10-30 ##
 * `gbc` & compiler library: 0.10.1.0

diff --git a/compiler/IO.hs b/compiler/IO.hs
@@ -7,6 +7,7 @@ module IO
     , parseASTFile
     , parseNamespaceMappings
     , parseAliasMappings
+    , slashNormalize
     )
     where
 
@@ -54,9 +55,10 @@ parseBondFile importDirs file = do
             Just path' -> do
                 content <- readFileUtf8 path'
                 return (path', content)
-            Nothing -> fail $ "Can't find import file " ++ importFile
+            Nothing -> fail $ "Can't find import file " ++ importFile'
       where
-        findFilePath dirs = fmap (</> importFile) <$> firstM (doesFileExist . (</> importFile)) dirs
+        importFile' = slashNormalize importFile
+        findFilePath dirs = fmap (</> importFile') <$> firstM (doesFileExist . (</> importFile')) dirs
 
     readFileUtf8 name = do
         h <- openFile name ReadMode
@@ -102,3 +104,15 @@ combinedMessage err = id $ T.unpack $ T.intercalate (T.pack ", ") messages
         -- parseErrorPretty returns a multi-line String.
         -- We need to break it up to make a useful one-line message.
         messages = T.splitOn (T.pack "\n") $ T.strip $ T.pack $ parseErrorTextPretty err
+
+-- | Normalizes a file path to only use the current platform's preferred
+-- directory separator.
+--
+-- Bond doesn't support files or directories with backslashes in their
+-- names, so backslashes are always converted to the platform's preferred
+-- separator.
+slashNormalize :: FilePath -> FilePath
+slashNormalize path = map replace path
+  where replace '/'  = pathSeparator
+        replace '\\' = pathSeparator
+        replace c    = c
diff --git a/compiler/Main.hs b/compiler/Main.hs
@@ -191,9 +191,8 @@ javaCodegen Java {..} = do
                                   -- code. This breaks compilation of generated
                                   -- code if either path has components that
                                   -- start with u.
-                                  backToForward s = map (\c -> if c == '\\' then '/' else c) s
-                                  safeBondFile = backToForward bondFile
-                                  safeJavaFile = backToForward javaFile
+                                  safeBondFile = slashForward bondFile
+                                  safeJavaFile = slashForward javaFile
 
                     createDir packageDir
                     LTIO.writeFile (packageDir </> javaFile) content

diff --git a/compiler/Options.hs b/compiler/Options.hs
@@ -1,7 +1,7 @@
 -- Copyright (c) Microsoft. All rights reserved.
 -- Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-{-# LANGUAGE DeriveDataTypeable #-}
+{-# LANGUAGE DeriveDataTypeable, RecordWildCards #-}
 {-# OPTIONS_GHC -fno-warn-missing-fields #-}
 {-# OPTIONS_GHC -fno-cse #-}
 
@@ -16,6 +16,7 @@ import Paths_bond (version)
 import Data.Version (showVersion)
 import System.Console.CmdArgs
 import System.Console.CmdArgs.Explicit (processValue)
+import IO (slashNormalize)
 
 data ApplyOptions =
     Compact |
@@ -130,14 +131,30 @@ schema = Schema
     name "schema" &=
     help "Output the JSON representation of the schema"
 
+slashNormalizeOption :: Options -> Options
+slashNormalizeOption Options = Options
+slashNormalizeOption o@Cpp{..}    = o { files = map slashNormalize files,
+                                        import_dir = map slashNormalize import_dir,
+                                        output_dir = slashNormalize output_dir }
+slashNormalizeOption o@Cs{..}     = o { files = map slashNormalize files,
+                                        import_dir = map slashNormalize import_dir,
+                                        output_dir = slashNormalize output_dir }
+slashNormalizeOption o@Java{..}   = o { files = map slashNormalize files,
+                                        import_dir = map slashNormalize import_dir,
+                                        output_dir = slashNormalize output_dir }
+slashNormalizeOption o@Schema{..} = o { files = map slashNormalize files,
+                                        import_dir = map slashNormalize import_dir,
+                                        output_dir = slashNormalize output_dir }
+
+
 mode :: Mode (CmdArgs Options)
 mode = cmdArgsMode $ modes [cpp, cs, java, schema] &=
     program "gbc" &=
     help "Compile Bond schema file(s) and generate specified output. The schema file(s) can be in one of two formats: Bond IDL or JSON representation of the schema abstract syntax tree as produced by `gbc schema`. Multiple schema files can be specified either directly on the command line or by listing them in a text file passed to gbc via @listfile syntax." &=
     summary ("Bond Compiler " ++ showVersion version ++ ", (C) Microsoft")
 
 getOptions :: IO Options
-getOptions = cmdArgsRun mode
+getOptions = slashNormalizeOption <$> cmdArgsRun mode
 
 processOptions :: [String] -> Options
 processOptions = cmdArgsValue . processValue mode
diff --git a/compiler/src/Language/Bond/Codegen/Cpp/Apply_h.hs b/compiler/src/Language/Bond/Codegen/Cpp/Apply_h.hs
@@ -35,7 +35,7 @@ namespace bond
 } // namespace bond
 |])
   where
-    includeImport (Import path) = [lt|#include "#{dropExtension path}_apply.h"|]
+    includeImport (Import path) = [lt|#include "#{dropExtension (slashForward path)}_apply.h"|]
 
     export_attr = optional (\a -> [lt|#{a}|]) export_attribute
 

diff --git a/compiler/src/Language/Bond/Codegen/Cpp/Grpc_h.hs b/compiler/src/Language/Bond/Codegen/Cpp/Grpc_h.hs
@@ -6,7 +6,7 @@
 module Language.Bond.Codegen.Cpp.Grpc_h (grpc_h) where
 
 import System.FilePath
-import Data.Maybe(isNothing)
+import Data.Maybe (isNothing)
 import Data.Monoid
 import Prelude
 import qualified Data.Text.Lazy as L
@@ -66,7 +66,7 @@ grpc_h export_attribute cpp file imports declarations = ("_grpc.h", [lt|
 
 |])
   where
-    includeImport (Import path) = [lt|#include "#{dropExtension path}_grpc.h"|]
+    includeImport (Import path) = [lt|#include "#{dropExtension (slashForward path)}_grpc.h"|]
 
     idl = MappingContext idlTypeMapping [] [] []
 

diff --git a/compiler/src/Language/Bond/Codegen/Cpp/Reflection_h.hs b/compiler/src/Language/Bond/Codegen/Cpp/Reflection_h.hs
@@ -37,7 +37,7 @@ reflection_h export_attribute cpp file imports declarations = ("_reflection.h",
     cppType = getTypeName cpp
 
     -- template for generating #include statement from import
-    include (Import path) = [lt|#include "#{dropExtension path}_reflection.h"|]
+    include (Import path) = [lt|#include "#{dropExtension (slashForward path)}_reflection.h"|]
 
     -- template for generating struct schema
     schema s@Struct {..} = [lt|//

diff --git a/compiler/src/Language/Bond/Codegen/Cpp/Types_h.hs b/compiler/src/Language/Bond/Codegen/Cpp/Types_h.hs
@@ -72,7 +72,7 @@ types_h export_attribute userHeaders enumHeader allocator alloc_ctors_enabled ty
 
     cppDefaultValue = CPP.defaultValue cpp
 
-    includeImport (Import path) = [lt|#include "#{dropExtension path}_types.h"|]
+    includeImport (Import path) = [lt|#include "#{dropExtension (slashForward path)}_types.h"|]
 
     optionalHeader (False, _) = mempty
     optionalHeader (True, header) = includeHeader header

diff --git a/compiler/src/Language/Bond/Codegen/Util.hs b/compiler/src/Language/Bond/Codegen/Util.hs
@@ -28,6 +28,7 @@ module Language.Bond.Codegen.Util
     , uniqueNames
     , indent
     , newLine
+    , slashForward
     ) where
 
 import Data.Int (Int64)
@@ -131,3 +132,9 @@ uniqueNames names reservedInit = reverse $ go names [] reservedInit
     go (name:remaining) acc reservedAcc = go remaining (newName:acc) (newName:reservedAcc)
       where
         newName = uniqueName name reservedAcc
+
+-- | Converts all file path slashes to forward slashes.
+slashForward :: String -> String
+slashForward path = map replace path
+  where replace '\\' = '/'
+        replace c    = c
diff --git a/compiler/tests/TestMain.hs b/compiler/tests/TestMain.hs
@@ -81,6 +81,11 @@ tests = testGroup "Compiler tests"
                 , "--enum-header"
                 ]
                 "with_enum_header"
+            , verifyCodegen
+                 [ "c++"
+                 , "--import-dir=tests/schema/imports"
+                 ]
+                 "import"
             , verifyCodegen
                 [ "c++"
                 , "--allocator=arena"
@@ -181,6 +186,11 @@ tests = testGroup "Compiler tests"
                 , "--namespace=tests=nsmapped"
                 ]
                 "basic_types_nsmapped"
+            , verifyCodegen
+                 [ "c#"
+                 , "--import-dir=tests/schema/imports"
+                 ]
+                 "import"
             , testGroup "Grpc"
                 [ verifyCsGrpcCodegen
                     [ "c#"
@@ -216,6 +226,11 @@ tests = testGroup "Compiler tests"
                 , "--namespace=tests=nsmapped"
                 ]
                 "basic_types_nsmapped"
+            , verifyCodegen
+                 [ "java"
+                 , "--import-dir=tests/schema/imports"
+                 ]
+                 "import"
             ]
         ]
     ]

diff --git a/compiler/tests/Tests/Codegen.hs b/compiler/tests/Tests/Codegen.hs
@@ -184,7 +184,7 @@ verifyFile options baseName typeMapping subfolder template =
     codegen = do
         aliasMapping <- parseAliasMappings $ using options
         namespaceMapping <- parseNamespaceMappings $ namespace options
-        (Bond imports namespaces declarations) <- parseBondFile [] $ "tests" </> "schema" </> baseName <.> "bond"
+        (Bond imports namespaces declarations) <- parseBondFile (import_dir options) $ "tests" </> "schema" </> baseName <.> "bond"
         let mappingContext = MappingContext typeMapping aliasMapping namespaceMapping namespaces
         let (_, code) = template mappingContext baseName imports declarations
         return $ BS.pack $ unpack code

diff --git a/compiler/tests/generated/alloc_ctors/import_reflection.h b/compiler/tests/generated/alloc_ctors/import_reflection.h
@@ -0,0 +1,50 @@
+
+#pragma once
+
+#include "import_types.h"
+#include <bond/core/reflection.h>
+#include "dir1/dir2/empty_reflection.h"
+
+namespace import_test
+{
+    //
+    // HasEmpty
+    //
+    struct HasEmpty::Schema
+    {
+        typedef ::bond::no_base base;
+
+        static const ::bond::Metadata metadata;
+
+        private: static const ::bond::Metadata s_e_metadata;
+
+        public: struct var
+        {
+            // e
+            typedef struct : ::bond::reflection::FieldTemplate<
+                0,
+                ::bond::reflection::optional_field_modifier,
+                HasEmpty,
+                ::empty::Empty,
+                &HasEmpty::e,
+                &s_e_metadata
+            > {}  e;
+        };
+
+        private: typedef boost::mpl::list<> fields0;
+        private: typedef boost::mpl::push_front<fields0, var::e>::type fields1;
+
+        public: typedef fields1::type fields;
+
+
+        static ::bond::Metadata GetMetadata()
+        {
+            return ::bond::reflection::MetadataInit("HasEmpty", "import_test.HasEmpty",
+                ::bond::reflection::Attributes()
+            );
+        }
+    };
+
+
+
+} // namespace import_test
diff --git a/compiler/tests/generated/alloc_ctors/import_types.cpp b/compiler/tests/generated/alloc_ctors/import_types.cpp
@@ -0,0 +1,15 @@
+
+#include "import_reflection.h"
+#include <bond/core/exception.h>
+
+namespace import_test
+{
+
+    const ::bond::Metadata HasEmpty::Schema::metadata
+        = HasEmpty::Schema::GetMetadata();
+
+    const ::bond::Metadata HasEmpty::Schema::s_e_metadata
+        = ::bond::reflection::MetadataInit("e");
+
+
+} // namespace import_test