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

Fix source code extraction when dealing with Unicode characters #178

Merged
merged 1 commit into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## [Unreleased]

- Fixed an issue where extracting code using `Review.Rule.withSourceCodeExtractor` would not get the correct source code when source contains Unicode characters.

## [2.14.0] - 2024-06-14

Support new visitors that visit "extra files".
Expand Down
56 changes: 41 additions & 15 deletions src/Review/Rule.elm
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ import Review.Project.InvalidProjectError as InvalidProjectError
import Review.Project.ProjectModule as ProjectModule exposing (OpaqueProjectModule)
import Review.Project.Valid as ValidProject exposing (ValidProject)
import Review.RequestedData as RequestedData exposing (RequestedData(..))
import Unicode
import Vendor.Graph as Graph exposing (Graph)
import Vendor.IntDict as IntDict
import Vendor.Zipper as Zipper exposing (Zipper)
Expand Down Expand Up @@ -5958,23 +5959,48 @@ visitCaseBranch caseBlockWithRange (( _, caseExpression ) as caseBranch) rules =


extractSourceCode : List String -> Range -> String
extractSourceCode lines range =
lines
|> List.drop (range.start.row - 1)
|> List.take (range.end.row - range.start.row + 1)
|> mapLast (String.slice 0 (range.end.column - 1))
|> String.join "\n"
|> String.dropLeft (range.start.column - 1)


mapLast : (a -> a) -> List a -> List a
mapLast mapper lines =
case List.reverse lines of
extractSourceCode lines { start, end } =
case List.drop (start.row - 1) lines of
[] ->
lines
""

first :: rest ->
List.reverse (mapper first :: rest)
firstLine :: rest ->
if start.row == end.row then
Unicode.slice
(start.column - 1)
(end.column - 1)
firstLine

else
let
{ linesTaken, lastLine } =
takeNLines (end.row - start.row - 1) rest ""
in
Unicode.dropLeft (start.column - 1) firstLine
++ linesTaken
++ (case lastLine of
Just lastLine_ ->
"\n" ++ Unicode.left (end.column - 1) lastLine_

Nothing ->
""
)


takeNLines : Int -> List String -> String -> { linesTaken : String, lastLine : Maybe String }
takeNLines n lines linesTaken =
if n <= 0 then
{ linesTaken = linesTaken, lastLine = List.head lines }

else
case lines of
[] ->
{ linesTaken = linesTaken, lastLine = Nothing }

line :: rest ->
takeNLines (n - 1)
rest
(linesTaken ++ "\n" ++ line)


type RuleProjectVisitor
Expand Down
24 changes: 24 additions & 0 deletions tests/NoNegationInIfConditionTest.elm
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,30 @@ a =
2
else
1
"""
]
, test "should correctly extract sources containing 2-part UTF-16 characters" <|
\() ->
"""module A exposing (..)
a =
if not condition then
"first🔧"
else
"second🌐"
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Don't use if expressions with negated conditions"
, details = [ "We at fruits.com think that if expressions are more readable when the condition is not negated." ]
, under = "not"
}
|> Review.Test.whenFixed """module A exposing (..)
a =
if condition then
"second🌐"
else
"first🔧"
"""
]
]
Loading