Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect LOC reported for syntax error #50

Open
bcardarella opened this issue Jan 21, 2024 · 8 comments
Open

Incorrect LOC reported for syntax error #50

bcardarella opened this issue Jan 21, 2024 · 8 comments
Assignees

Comments

@bcardarella
Copy link
Contributor

bcardarella commented Jan 21, 2024

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
Copy link
Contributor Author

This is on Elixir 1.16

@NduatiK
Copy link
Contributor

NduatiK commented Jan 24, 2024

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: elixir-lang/elixir#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
Copy link
Contributor Author

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

@NduatiK
Copy link
Contributor

NduatiK commented Jan 24, 2024

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
Copy link
Contributor

NduatiK commented Jan 24, 2024

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 (elixir-lang/elixir#8605) makes me believe it would only affect the runtime code and not the compiler warnings

@bcardarella
Copy link
Contributor Author

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

@NduatiK
Copy link
Contributor

NduatiK commented Jan 24, 2024

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
Copy link
Contributor Author

I'll review it later.

@AZholtkevych AZholtkevych moved this from Todo to On Hold in LiveView Native Jan 25, 2024
@AZholtkevych AZholtkevych moved this from On Hold to Todo in LiveView Native Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants