Zaid-Ajaj / Npgsql.FSharp.Analyzer

F# analyzer that provides embedded SQL syntax analysis, type-checking for parameters and result sets and nullable column detection when writing queries using Npgsql.FSharp.
MIT License
137 stars 9 forks source link

Analyzer doesn't seem to detect referenced literal queries in nested modules in fsx files #35

Open jkone27 opened 3 years ago

jkone27 commented 3 years ago

Describe the bug I followed the setup for the analyzer in ionide, windows 10 and net5, but I cannot make it work...

if i change the db name the analyzer complains, so he is alive (?) but he cannot check issues with db columns... image

docker run --name localdb -e POSTGRES_PASSWORD=password -d -p 5432:5432 postgres

in postgres i then run this

CREATE SCHEMA test;

CREATE TABLE test.articles (
    id BIGINT PRIMARY KEY NOT NULL, --could be string to be extensible
    art_name CITEXT UNIQUE NOT NULL, --creates unique index
    stock BIGINT NOT NULL
);

i installed pgsql analyzer (see sql errors at edit time) at root of where the .fsx file is

dotnet tool install --global Paket
paket init
paket add NpgsqlFSharpAnalyzer --group Analyzers

and the NPGSQL_FSHARP file (which seems to work as wrong connection triggers)

Host=localhost; Username=postgres; Password=password; Database=postgres

i also tried to run Ubik on the file, but it reports no error, while the column name x does not exist in practice.

To Reproduce

#r "nuget:Npgsql.FSharp"
#r "nuget:FSharp.Data"

open FSharp.Data
open FSharp.Core

module Constants =

    [<Literal>]
    let sqlDevConnection = "Host=localhost;Database=postgres;Username=postgres;Password=password"

//sensitive data should be injected from environment
System.Environment.SetEnvironmentVariable("DATABASE_CONNECTION_STRING", Constants.sqlDevConnection)

module Domain =
    type Article = { 
            Id : int ; 
            Name: string; 
            Stock: int 
        } 

module Data =
    open ProvidedTypes
    open Domain
    open Npgsql.FSharp

    module Queries =

        [<Literal>]
        let getArticles =
            "SELECT x \n\
            FROM test.articles"

    let getConnectionString () =
        System.Environment.GetEnvironmentVariable "DATABASE_CONNECTION_STRING"

    let getArticles () =
        getConnectionString()
        |> Sql.connect
        |> Sql.query Queries.getArticles
        |> Sql.execute (fun read ->
            {
                Id = read.int "id"
                Name = read.text "art_name"
                Stock = read.int "stock"
            })

Expected behavior Should underline the non existing column X and give a warning

Screenshots

image

image

image

Desktop (please complete the following information):

Zaid-Ajaj commented 3 years ago

Hi there @jkone27 thanks for filing the issue. My initial thoughts is that the analyser isn't able to detect the referenced query using the fully qualified name Queries.getArticles. can you try just getArticles without the full module? Or simply use an inline query string to see If that fixes the issue?

I will give this a try later and will let you know how it goes

Zaid-Ajaj commented 3 years ago

You can also use triple quotes when writing multi-line SQL queries without having to all \n to each line

jkone27 commented 3 years ago

Tried your suggestions but I still was unlucky.. no error for non existing x column image

jkone27 commented 3 years ago

The issue is related to nested modules in FSI

image

when there is nested modules, either in the same module or in an external module, query string resolution doesn't seem to work

Zaid-Ajaj commented 3 years ago

Proposed solution is enhance the way we are detecting the literal queries: traverse the AST and detect this pattern

[<Literal>]
let  query = "<SQL>"

while keeping track of the original module, so

module Queries = 
  let [<Literal> getFiles = "SELECT * FROM films"

Should keep track of the fact that getFiles is coming from Queries etc. The same should hold for deeply nested queries too

Zaid-Ajaj commented 3 years ago

Since this is quite difficult to implement, it is easier to just not use nested modules in fsx queries and to keep things simple