liveview-native / live_view_native_stylesheet

MIT License
10 stars 4 forks source link

Incorrect LOC reported for syntax error #50

Open bcardarella opened 7 months ago

bcardarella commented 7 months ago

Using the following stylesheet:

defmodule LvnWeb.AppStyles do
  use LiveViewNative.Stylesheet, :swiftui

  ~SHEET"""
  "clip:circle" do
    clipShape(.circle)
  end

  "image-scale:" <> size do
    imageScale(.large)
  end
  """
end

and I get the following error:

Compiling 3 files (.ex)
    warning: variable "size" is unused (if the variable is not meant to be used, prefix it with an underscore)
    │
  5 │   "clip:circle" do
    │   ~~~~~~~~~~~~~~~~
    │
    └─ (lvn 0.1.0) lib/lvn_web/styles/app_styles.ex:5: LvnWeb.AppStyles.class/2

the warning should be reported for the 2nd block, not the first.

bcardarella commented 7 months ago

This is on Elixir 1.16

NduatiK commented 7 months ago

I tested this using the the stylesheet repo's MockSheet. On my end, the error shows up at ~SHEET""" (also 1.16)

This might be unavoidable based on Jose's response to this issue: https://github.com/elixir-lang/elixir/issues/8605. ~SHEET is syntactic sugar for a sigil_SHEET("...") and any warning generated inside the macro will be located at the caller.

I'll need to investigate why the warning shows up on line 5 instead of on the ~SHEET on line 4 for the SwiftUI parser.

bcardarella commented 7 months ago

can't we get he location of ~SHEET and count \ns for location offsets?

NduatiK commented 7 months ago

My point here is not that it is difficult to for the parser to generate the error, it's that the Elixir compile might not support accurate locations on macro code.

If we can suppress the Elixir compiler warning, then we can generate our own.

By the way, this is based on some quick research on compiler warnings. I'll continue to look into it when I get some time.

NduatiK commented 7 months ago

One more idea is to set generated: true for the quote that generates the warning:

:generated - marks the given chunk as generated so it does not emit warnings. It is also useful to avoid dialyzer reporting errors when macros generate unused clauses. https://hexdocs.pm/elixir/Kernel.SpecialForms.html#quote/2

The :line options might also be useful, but the conversation I linked to earlier (https://github.com/elixir-lang/elixir/issues/8605) makes me believe it would only affect the runtime code and not the compiler warnings

bcardarella commented 7 months ago

we don't want to be emitting warnings in the build, I would have thought that the LOC offsets would be sufficient here

NduatiK commented 7 months ago

LOC offsets

What do we do with the offsets? We know from the tests that the unused variable already has the correct line in its metadata. What more can we do?

bcardarella commented 7 months ago

I'll review it later.