Linter that spots passing PS objects to the FFI

Chat
#1

Does anyone have a linter they use that spots when a PS object is passed to the FFI, either directly or as part of a Record/Array? Say for example one does:

data Foo = A | B
foreign foo :: Foo -> Int
z = foo A

This of course is perfectly valid PureScript, but often reflects an error where someone assumed a PureScript object would be converted to a string or, if it is a constructor, its value would be passed to the FFI (ie if it was data MyString = MyString String that String would go to the FFI if one passed a MyString String). That sort of programming error is difficult to spot. If anyone has a linter that could pick up on those things, it’d be super helpful!

1 Like
#2

Is it possible to do this robustly without typechecking first? For example, polymorphic FFI declarations.

#3

I’m not sure how polymorphic FFI declarations would help in this case, although I’m curious to hear more.

The goal is to have a linting rule prohibiting someone from writing:

newtype MyInt = MyInt Int
foreign doSomething :: MyInt -> Boolean

and perhaps suggesting:

foreign doSomething :: Int -> Boolean

So basically, with this rule applied, the FFI would only be allowed plain types like Int or Boolean, arrays/records, and types that are foreign themselves, like Foreign or Object or a Window. I realize that in many cases folks do want to pass PureScript-created types to the FFI (ie passing a Maybe Int to the FFI), but it would be nice to be able to prohibit these with a linter.

#4

I think Nate meant if you had a polymorphic type like

foreign import myIdentity :: forall a. a -> a

what would the linter do? Would it allow or disallow such a type? a could be instantiated with a JS primitive type or a PS type

#5

I don’t think this is worth doing because I want to pass PS types often, but this could be achieved with some language support I think?

-- compiler only instances class, or requires unsafeCoerce for users to implement
class Foreign a

instance foreignInt :: Foreign Int

-- ...

instance foreignFn ::
  ( Foreign a
  , Foreign b
  ) => Foreign (a -> b)

data Foo = Foo Int

foreign import foo :: Foo -> Int

-- the actual type of foo (the dict discard is added automatically, or we use ErasedConstraint)
-- foo :: Foreign (Foo -> Int) => Foo -> Int

-- Error: No instance found for Foreign Foo
test1 :: Int
test1 = foo (Foo 0)

unsafeForeign :: forall t r. Proxy t => (Foreign t => r) -> r
unsafeForeign _ f = (coerce f) {}
  where
    coerce :: (Foreign t => r) -> ({} -> r)
    coerce = unsafeCoerce

-- Works
test2 :: Int
test2 = unsafeForeign (Proxy :: Proxy Foo) (foo (Foo 0))
#6

You could do it without compiler support actually, but it’s a little more verbose

foreign import data Foreign :: Type -> Type

class IsForeign a

instance isForeignInt :: IsForeign Int

-- ...

instance isForeignFn ::
  ( IsForeign a
  , IsForeign b
  ) => IsForeign (a -> b)


runForeign :: forall a. IsForeign a => Foreign a -> a
runForeign frn = unsafeCoerce frn

unsafeForeign :: forall t r. Proxy t => (IsForeign t => r) -> r
unsafeForeign _ f = (coerce f) {}
  where
    coerce :: (IsForeign  => r) -> ({} -> r)
    coerce = unsafeCoerce

data Foo = Foo Int

foreign import foo :: Foreign (Foo -> Int)

-- No instance found error
test1 :: Int
test1 = runForeign foo (Foo 0)

test2 :: Int
test2 = unsafeForeign (Proxy :: Proxy Foo) (runForeign foo (Foo 0))

You could then run a linter that just warns on using unsafeForeign and writing foreign imports that aren’t explicitly wrapped in the Foreign constructor

1 Like
#7

The motivation comes from our code base at Meeshkan - while we try to catch these in code review, sometimes they slip through and cause nasty bugs. So if I can make a linter that can run as part of the CI pipeline, that’d help.

#8

This looks great! Thanks a lot for the suggestion.

2 Likes
#9

Ah sorry @natefaubion I misread your comment. Yeah, the linter would also have to forbid polymorphic FFI declarations.

#10

Curiously, could solving this on the JS toolchain end handle this?

Wondering if using/emitting typescript/flow code from the purescript side would help.

Perhaps together with https://github.com/purescript/purescript/issues/4020 . It can make out for a nice js/ts interop.

1 Like
#11

I never considered the use of a newtype in a FFI declaration to be a problem, but this thread raised a bit of doubt for me. So I asked about it in Discord and received some good answers: Discord

The TL;DR is: Don’t worry too much about it, but do be aware that the compiler won’t help you avoid a bug if you change the newtype to an ADT without making additional changes.

1 Like
#12

newtypes have been really useful, agreed. The issue has come up when using an ADT & it hits the FFI. Sometimes it’s easy to accidentally convert a newtype to an ADT or vice-versa when doing refactoring.