rescript-lang / reanalyze

Experimental analyses for ReScript and OCaml: globally dead values/types, exception analysis, and termination analysis.
MIT License
275 stars 20 forks source link

[Feature request] Integrate with `@deriving({abstract: light})` #183

Open mvaled opened 2 years ago

mvaled commented 2 years ago

All the fields in the following script are reported as dead:

@deriving({abstract: light})
type person = {
  name: string,
  age: int,
}

let joe = person(~name="Joe", ~age=20)
let joeName = name(joe)

Report:

  Warning Dead Type
  File "src/TestBug.res", line 3, characters 2-14
  person.name is a record label never used to read a value
  <-- line 3
    @dead("person.name") name: string,

  Warning Dead Type
  File "src/TestBug.res", line 4, characters 2-10
  person.age is a record label never used to read a value
  <-- line 4
    @dead("person.age") age: int,

However, only age is not actually used in the code, while name shouldn't be reported.

cristianoc commented 2 years ago

Deriving abstract is expected to be used less and less in future. Either objects, or records, in the latest compiler versions, should be able to express all the needed functionality.

mvaled commented 2 years ago

We use it heavily in a custom integration we have of a React-like framework based on Mithril -- this is mainly because our app is offline-first and the connection is very limited; so we wanted a really small library.

This, is for instance a very small component:

open Prelude
open Mithril
open CompletionStatus

@deriving({abstract: light})
type makeProps = {
  @optional completion: completionInformation,
  @optional children: element,
}

let make = _ => {
  component()->view(vnode => {
    let completion = vnode.attrs->completion->default(CompletionStatus.Defaults.completion)
    let {totalNested, totalInspected, isOverdue} = completion

    <div className="tile-action">
      {show(
        if completion->isFullyInspected {
          <Feather icon=#check_circle className="text-success" />
        } else {
          <span className={isOverdue ? "text-error" : ""}>
            {show(totalInspected->Int.toString ++ "/" ++ totalNested->Int.toString)}
          </span>
        },
      )}
    </div>
  })
}

And it can be used like <ShowCompletionInfo completion={...}/>.


It would require some rethinking to remove the dependency on abstract.

cristianoc commented 2 years ago

@mattdamon108 this seems doable with JSX V4?

mununki commented 2 years ago

@mattdamon108 this seems doable with JSX V4?

Yes, I think so too.

@mvaled Can you try the latest compiler and rescript-react as posted on the forum? https://forum.rescript-lang.org/t/call-for-help-2-test-jsx-v4/3781 Maybe something like:

type props<'completion, 'children> = {
  completion: 'completion,
  children: 'children
}
let make = props => {
  let {completion, children} = props

  component()->view(vnode => {
    let completion = vnode.attrs->completion->default(CompletionStatus.Defaults.completion)
    let {totalNested, totalInspected, isOverdue} = completion

    <div className="tile-action">
      {show(
        if completion->isFullyInspected {
          <Feather icon=#check_circle className="text-success" />
        } else {
          <span className={isOverdue ? "text-error" : ""}>
            {show(totalInspected->Int.toString ++ "/" ++ totalNested->Int.toString)}
          </span>
        },
      )}
    </div>
  })
}
mununki commented 2 years ago

If it is not working then try this:

type props = {
  completion: completionInformation,
  children: element
}
mvaled commented 2 years ago

@mattdamon108

@mvaled Can you try the latest compiler and rescript-react as posted on the forum? https://forum.rescript-lang.org/t/call-for-help-2-test-jsx-v4/3781

Thanks.

One question, though. Would this require the actual React JS library or it's just the transformation, that would work with any compatible "react" binding? e.g Our rescript-mithril instead of '@rescript/react'.

If it's just the transformation, then I'll try to upgrade our Mithril bindings to be compatible with JSX v4.

mununki commented 2 years ago

I just checked about Mithril, never knew before. You made your own bindings! How about your bsconfig.json? Do you use "reason"."react-jsx" option?

mununki commented 2 years ago

I've looked into your bindings. I think it would be working highly possibility. You can check the latest rescript-react binding and update your Mithrill bindings accordingly, then try with latest compiler's JSX v4.

mununki commented 2 years ago

One question, though. Would this require the actual React JS library or it's just the transformation, that would work with any compatible "react" binding?

Just transformation and working with react binding.

mvaled commented 2 years ago

How about your bsconfig.json? Do you use "reason"."react-jsx" option?

None, actually. This is our whole bsconfig.json:

{
  "name": "kaiko-survey-tool",
  "sources": [
    {
      "dir": "src/",
      "subdirs": true
    }
  ],
  "suffix": ".js",
  "reason": {
    "react-jsx": 3
  },
  "bs-dependencies": [
    "@kaiko/reindexed",
    "rescript-webapi",
    "@ryyppy/rescript-promise",
    "rescript-mithril",
    "bs-fetch",
    "rescript-prelude",
    "rescript-deser",
    "rescript-action-queue"
  ],
  "package-specs": {
    "module": "es6"
  },
  "warnings": {
    "error": "+8+11+26+27+33+56"
  }
}
mununki commented 2 years ago

bsconfig seems you're using jsx transformation already. Yes, it's good sign. Good luck!

mununki commented 2 years ago

When you try the latest compiler with JSX v4, change this property in bsconfig.json

"reason": {
    "react-jsx": 3
  },

to

"jsx": {
  "version": 4
}
mvaled commented 2 years ago

bsconfig seems you're using jsx transformation already

Right! Somehow my eyes skip over those lines 😄