Merge pull request #2300 from nguerrera/format-interop-guidelines

Format interop guidelines doc for easier text editing

Author: stephentoub

Documentation/coding-guidelines/interop-guidelines.md

@@ -5,39 +5,73 @@ Interop Guidelines
 We have the following goals related to interop code being used in CoreFX:
 
 - Minimize code duplication for interop.
-	- We should only define a given interop signature in a single place.  This stuff is tricky, and we shouldn’t be copy-and-pasting it.
-- Minimize unnecessary IL in assemblies.  
-	- Interop signatures should only be compiled into the assemblies that actually consume them.  Having extra signatures bloats assemblies and makes it more difficult to do static analysis over assemblies to understand what they actually use.  It also leads to problems when such static verification is used as a gate, e.g. if a store verifies that only certain APIs are used by apps in the store.
+  - We should only define a given interop signature in a single place.
+    This stuff is tricky, and we shouldn't be copy-and-pasting it.
+- Minimize unnecessary IL in assemblies.
+  - Interop signatures should only be compiled into the assemblies that
+    actually consume them. Having extra signatures bloats assemblies and
+    makes it more difficult to do static analysis over assemblies to
+    understand what they actually use. It also leads to problems when such
+    static verification is used as a gate, e.g. if a store verifies that
+    only certain APIs are used by apps in the store.
 - Keep interop code isolated and consolidated.
-	- This is both for good hygiene and to help keep platform-specific code separated from platform-neutral code, which is important for maximizing reusable code above PAL layers.
+  - This is both for good hygiene and to help keep platform-specific code
+    separated from platform-neutral code, which is important for maximizing
+    reusable code above PAL layers.
 
 ## Approach
 
 ### Interop type
-- All code related to interop signatures (DllImports, interop structs used in DllImports, constants that map to native values, etc.) should live in a partial, static, and internal “Interop” class in the root namespace, e.g.
+- All code related to interop signatures (DllImports, interop structs
+  used in DllImports, constants that map to native values, etc.) should
+  live in a partial, static, and internal “Interop” class in the root
+  namespace, e.g.
+
 ```C#
-internal static partial class Interop { … }
+internal static partial class Interop { ... }
 ```
 
-- Declarations shouldn’t be in Interop directly, but rather within a partial, static, internal nested type named for a given library or set of libraries, e.g.
+- Declarations shouldn't be in Interop directly, but rather within a
+  partial, static, internal nested type named for a given library or set
+  of libraries, e.g.
+
 ```C#
 internal static partial class Interop
 {
-    internal static partial class libc { … }
+    internal static partial class libc { ... }
 }
 ...
 internal static partial class Interop
 {
-    internal static partial class mincore { … }
+    internal static partial class mincore { ... }
 }
 ```
-- With few exceptions, the only methods that should be defined in these interop types are DllImports.
-	- Exceptions are limited to times when most or every consumer of a particular DllImport will need to wrap its invocation in a helper, e.g. to provide additional marshaling support, to hide thread-safety issues in the underlying OS implementation, to do any required manipulation of safe handles, etc.  In such cases, the DllImport should be private whenever possible rather than internal, with the helper code exposed to consumers rather than having the DllImport exposed directly.
+- With few exceptions, the only methods that should be defined in these
+  interop types are DllImports.
+  - Exceptions are limited to times when most or every consumer of a
+  particular DllImport will need to wrap its invocation in a helper, e.g.
+  to provide additional marshaling support, to hide thread-safety issues
+  in the underlying OS implementation, to do any required manipulation of
+  safe handles, etc. In such cases, the DllImport should be private
+  whenever possible rather than internal, with the helper code exposed to
+  consumers rather than having the DllImport exposed directly.
 
 ### File organization
-- The Interop partial class definitions should live in Interop.*.cs files.  These Interop.*.cs files should all live under Common rather than within a given assembly’s folder.
-	- The only exception to this should be when an assembly P/Invokes to its own native library that isn’t available to or consumed by anyone else, e.g. System.IO.Compression P/Invoking to clrcompression.dll.  In such cases, System.IO.Compression should have its own Interop folder which follows a similar scheme as outlined in this proposal, but just for these private P/Invokes.
-- Under Common\src\Interop, we’ll have a folder for each target platform, and within each platform, for each library from which functionality is being consumed.  The Interop.*.cs files will live within those library folders, e.g.
+
+- The Interop partial class definitions should live in Interop.*.cs
+  files. These Interop.*.cs files should all live under Common rather than
+  within a given assembly's folder.
+  - The only exception to this should be when an assembly P/Invokes to its
+    own native library that isn't available to or consumed by anyone else,
+    e.g. System.IO.Compression P/Invoking to clrcompression.dll. In such
+    cases, System.IO.Compression should have its own Interop folder which
+    follows a similar scheme as outlined in this proposal, but just for
+    these private P/Invokes.
+- Under Common\src\Interop, we'll have a folder for each target
+  platform, and within each platform, for each library from which
+  functionality is being consumed. The Interop.*.cs files will live within
+  those library folders, e.g.
+
 ```
 \Common\src\Interop
     \Windows
@@ -50,11 +84,19 @@ internal static partial class Interop
         \libc
             ... interop files
 ```
-As shown above, platforms may be additive, in that an assembly may use functionality from multiple folders, e.g. System.IO.FileSystem’s Linux build will use functionality both from Unix (common across all Unix systems) and from Linux (specific to Linux and not available across non-Linux Unix systems).
+
+As shown above, platforms may be additive, in that an assembly may use functionality from multiple folders, e.g. System.IO.FileSystem's Linux build will use functionality both from Unix (common across all Unix systems) and from Linux (specific to Linux and not available across non-Linux Unix systems).
 
-- Interop.*.cs files are created in a way such that every assembly consuming the file will need every DllImport it contains.
-	- If multiple related DllImports will all be needed by every consumer, they may be declared in the same file, named for the functionality grouping, e.g. Interop.IOErrors.cs.
-	- Otherwise, in the limit (and the expected case for most situations) each Interop.*.cs file will contain a single DllImport and associated interop types (e.g. the structs used with that signature) and helper wrappers, e.g. Interop.strerror.cs.
+- Interop.*.cs files are created in a way such that every assembly
+  consuming the file will need every DllImport it contains.
+  - If multiple related DllImports will all be needed by every consumer,
+    they may be declared in the same file, named for the functionality
+    grouping, e.g. Interop.IOErrors.cs.
+  - Otherwise, in the limit (and the expected case for most situations)
+    each Interop.*.cs file will contain a single DllImport and associated
+    interop types (e.g. the structs used with that signature) and helper
+    wrappers, e.g. Interop.strerror.cs.
+
 ```
 \Common\src\Interop
     \Unix
@@ -65,8 +107,14 @@ As shown above, platforms may be additive, in that an assembly may use functiona
             \Interop.OutputDebugString.cs
 ```
 
-- If structs/constants will be used on their own without an associated DllImport, or if they may be used with multiple DllImports not in the same file, they should be declared in a separate file.
-- In the case of multiple overloads of the same DllImport (e.g. some overloads taking a SafeHandle and others taking an IntPtr, or overloads taking different kinds of SafeHandles), if they can’t all be declared in the same file (because they won’t all be consumed by all consumers), the file should be qualified with the key differentiator, e.g.
+- If structs/constants will be used on their own without an associated
+  DllImport, or if they may be used with multiple DllImports not in the
+  same file, they should be declared in a separate file.
+- In the case of multiple overloads of the same DllImport (e.g. some
+  overloads taking a SafeHandle and others taking an IntPtr, or overloads
+  taking different kinds of SafeHandles), if they can't all be declared in
+  the same file (because they won't all be consumed by all consumers), the
+  file should be qualified with the key differentiator, e.g.
 
 ```
 \Common\src\Interop
@@ -76,7 +124,12 @@ As shown above, platforms may be additive, in that an assembly may use functiona
             \Interop.DuplicateHandle_IntPtr.cs
 ```
 
-- The library names used per-platform are stored in internal constants in the Interop class in a private Libraries class in a per-platform file named Interop.Libraries.cs.  These constants are then used for all DllImports to that library, rather than having the string duplicated each time, e.g.
+- The library names used per-platform are stored in internal constants
+  in the Interop class in a private Libraries class in a per-platform file
+  named Interop.Libraries.cs. These constants are then used for all
+  DllImports to that library, rather than having the string duplicated
+  each time, e.g.
+
 ```C#
 internal static partial class Interop // contents of Common\src\Interop\Windows\Interop.Libraries.cs
 {
@@ -92,8 +145,11 @@ internal static partial class Interop // contents of Common\src\Interop\Windows\
         ...
     }
 }
+
 ```
-(Note that this will likely result in some extra constants defined in each assembly that uses interop, which minimally violates one of the goals, but it’s very minimal.)
+(Note that this will likely result in some extra constants defined in
+each assembly that uses interop, which minimally violates one of the
+goals, but it's very minimal.)
 
 - .csproj project files then include the interop code they need, e.g.
 ```XML
@@ -106,30 +162,45 @@ internal static partial class Interop // contents of Common\src\Interop\Windows\
     <Compile Include="Interop\Unix\libc\Interop.close.cs" />
     <Compile Include="Interop\Unix\libc\Interop.snprintf.cs" />
     ...
-</ItemGroup
+</ItemGroup>
 ```
 
 ### Build System
-When building CoreFx, we use the "OSGroup" property to control what target platform we are building for.  The valid values for this property are Windows_NT (which is the default value from MSBuild when running on Windows), Linux and OSX.
+When building CoreFx, we use the "OSGroup" property to control what
+target platform we are building for. The valid values for this property
+are Windows_NT (which is the default value from MSBuild when running on
+Windows), Linux and OSX.
 
-The build system sets a few MSBuild properties, depending on the OSGroup setting:
+The build system sets a few MSBuild properties, depending on the OSGroup
+setting:
 
 * TargetsWindows
 * TargetsLinux
 * TargetsOSX
 * TargetsUnix
 
-TargetsUnix is true for both OSX and Linux builds and can be used to include code that can be used on both Linux and OSX (e.g. it is written against a POSIX API that is present on both platforms).
+TargetsUnix is true for both OSX and Linux builds and can be used to
+include code that can be used on both Linux and OSX (e.g. it is written
+against a POSIX API that is present on both platforms).
 
-You should not test the value of the OSGroup property directly, instead use one of the values above.
+You should not test the value of the OSGroup property directly, instead
+use one of the values above.
 
 #### Project Files
-Whenever possible, a single .csproj should be used per assembly, spanning all target platforms, e.g. System.Console.csproj includes conditional entries for when targeting Windows vs when targeting Linux.  A property can be passed to msbuild to control which flavor is built, e.g. msbuild /p:OSGroup=OSX System.Console.csproj.
+Whenever possible, a single .csproj should be used per assembly,
+spanning all target platforms, e.g. System.Console.csproj includes
+conditional entries for when targeting Windows vs when targeting Linux.
+A property can be passed to msbuild to control which flavor is built,
+e.g. msbuild /p:OSGroup=OSX System.Console.csproj.
 
 ### Constants
-- Wherever possible, constants should be defined as "const".  Only if the data type doesn’t support this (e.g. IntPtr) should they instead be static readonly fields.
+- Wherever possible, constants should be defined as "const". Only if the
+  data type doesn't support this (e.g. IntPtr) should they instead be
+  static readonly fields.
+
+- Related constants should be grouped under a partial, static, internal
+  type, e.g. for error codes they'd be grouped under an Errors type:
 
-- Related constants should be grouped under a partial, static, internal type, e.g. for error codes they'd be grouped under an Errors type:
 ```C#
 internal static partial class Interop
 {
@@ -153,8 +224,23 @@ internal static partial class Interop
     }
 }
 ```
-Using enums instead of partial, static classes can lead to needing lots of casts at call sites and can cause problems if such a type needs to be split across multiple files (enums can’t currently be partial).  However, enums can be valuable in making it clear in a DllImport signature what values are permissible.  Enums may be used in limited circumstances where these aren’t concerns: the full set of values can be represented in the enum, and the interop signature can be defined to use the enum type rather than the underlying integral type.
+
+Using enums instead of partial, static classes can lead to needing lots
+of casts at call sites and can cause problems if such a type needs to be
+split across multiple files (enums can't currently be partial). However,
+enums can be valuable in making it clear in a DllImport signature what
+values are permissible. Enums may be used in limited circumstances where
+these aren't concerns: the full set of values can be represented in the
+enum, and the interop signature can be defined to use the enum type
+rather than the underlying integral type.
 
 ## Naming
-- Interop signatures / structs / constants should be defined using the same name / capitalization / etc. that’s used in the corresponding native code.
-	- We should not rename any of these based on managed coding guidelines.  The only exception to this is for the constant grouping type, which should be named with the most discoverable name possible; if that name is a concept (e.g. Errors), it can be named using managed naming guidelines.
+
+- Interop signatures / structs / constants should be defined using the
+  same name / capitalization / etc. that's used in the corresponding
+  native code.
+  - We should not rename any of these based on managed coding guidelines.
+    The only exception to this is for the constant grouping type, which
+    should be named with the most discoverable name possible; if that name
+    is a concept (e.g. Errors), it can be named using managed naming
+    guidelines.