onflow / cadence-tools Goto Github PK

Issue To Be Solved

Currently, duplicate cases are allowed in switch-statement. That can leave unreachable codes without the user being aware.

e.g.:

pub fun test(s: String) {
    switch s {
        case "foo":
            return 1
        case "bar":
            return 2
        case "bar":    // a duplicate case: no errors/warnings are reported
            return 3
    }
}

Suggest A Solution

Validate and reject duplicate cases during compile time.

As described in this comment, the default values are missing from the testing framework, and it will be a nice addition to have them available.

Problem

LS returns duplicate error in playground.

Steps to Reproduce

• Visit https://play.onflow.org/
• Click on the first account
• Delete the last character from the String type on line 22

(Discovered in onflow/flow-playground#288 and doesn't appear to be related to the playground code)

LS logs from https://github.com/onflow/flow-playground/blob/staging/src/util/language-client.ts#L63

{
    "method": "textDocument/publishDiagnostics",
    "params": {
        "uri": "inmemory://model/4",
        "diagnostics": [
            {
                "range": {
                    "start": {
                        "line": 21,
                        "character": 29
                    },
                    "end": {
                        "line": 21,
                        "character": 34
                    }
                },
                "severity": 1,
                "message": "cannot find type in this scope: `Strin`. not found in this scope"
            },
            {
                "range": {
                    "start": {
                        "line": 21,
                        "character": 29
                    },
                    "end": {
                        "line": 21,
                        "character": 34
                    }
                },
                "severity": 1,
                "message": "cannot find type in this scope: `Strin`. not found in this scope"
            }
        ]
    },
    "jsonrpc": "2.0"
}

[
    {
        "resource": {
            "$mid": 1,
            "external": "inmemory://model/3",
            "path": "/3",
            "scheme": "inmemory",
            "authority": "model"
        },
        "owner": "default",
        "severity": 8,
        "message": "cannot find type in this scope: `Strin`. not found in this scope",
        "startLineNumber": 22,
        "startColumn": 30,
        "endLineNumber": 22,
        "endColumn": 35
    },
    {
        "resource": {
            "$mid": 1,
            "external": "inmemory://model/3",
            "path": "/3",
            "scheme": "inmemory",
            "authority": "model"
        },
        "owner": "default",
        "severity": 8,
        "message": "cannot find type in this scope: `Strin`. not found in this scope",
        "startLineNumber": 22,
        "startColumn": 30,
        "endLineNumber": 22,
        "endColumn": 35
    }
]

This issue references vscode issue: onflow/vscode-cadence#204

Codeless generates invalid windows file paths.

Issue to be solved

At a resource that has more than one function, the comments for the second and further functions are not fully generated (they do not include the description and the param/return section)

e.g.:
source code -> https://github.com/onflow/flow-nft/blob/f8d716eb98c8d8eee1f45efb3eeaf01831312192/contracts/NonFungibleToken.cdc#L163
generated doc -> https://github.com/onflow/flow-nft/blob/default-implenentations/docs/NonFungibleToken/NonFungibleToken_Collection.md

Crashes when trying to return an array/dictionary/composite typed value from a script.

import Test

pub fun test() {
    var blockchain = Test.newEmulatorBlockchain()
    var result = blockchain.executeScript("pub fun main(): [UInt64] { return [1, 2, 3]}", [])

Root cause: Storage is not properly initialized

Description

Ensure capabilities cannot be accessed by unauthorized parties. For example, capabilities should not be accessible through a public field, including public dictionaries or arrays. Exposing a capability in such a way allows anyone to borrow it and perform all actions that the capability allows.

Acceptance criteria

• Linter issues a warning when capabilities are stored in public arrays or dictionaries
• Link to the guidance on what risks this poses

Issue To Be Solved

Not sure where this lands, but something like this would be nice

if I have code like below:

var a = "deniz"

some tool converting this to:

var a : String = "deniz"

Extension is showing types, but also would nice to people use this somehow, then we have this on deployed contracts.

Update AddContract to support latest CLI changes: onflow/flow-cli#908

Issue To Be Solved

It is currently not possible to import the Test contract:

Context

https://github.com/orgs/onflow/discussions/1330

Feature Request

Capabilities should not be accessible by unauthorized parties. For example, capabilities should not be accessible through a public field, including public dictionaries or arrays. Exposing a capability in such a way allows anyone to borrow it and perform all actions that the capability allows.

Detect and report public fields with a capability type; directly, or indirectly (e.g. array/dictionary of capabilities)

Issue To Be Solved

Developers may attempt to use a field or function of an optional type directly, e.g.

struct S {
   let x: Int
   // initializer omitted
}

let s: S? = S(x: 1)
s.x  // invalid: type `S?` has no field `x`.

However, the underlying type of the optional may have the requested member.

Currently, whenever the accessed member cannot be found on a type, an error is reported.

Suggested Solution

When the type of the accessed value is an optional type, check if it has a member with the requested member and suggest optional chaining to the user.

For instance suggest to use s?.x in the example above.

Problem

Currently, when the Test.newEmulatorBlockchain() is called, it returns a new Test.Blockchain struct. However, the underlying backend implementation is the same for all Test.Blockchain structs that are created in a single test file. This is due to the fact that the ContractValueHandler injects only once the backend implementation, when it first encounters the Test contract in a test file.

Steps to Reproduce

import Test

pub let blockchain = Test.newEmulatorBlockchain()
pub let account = blockchain.createAccount()

pub fun testEventRetrieval() {
    let evts = blockchain.events()
    Test.assert(evts.length == 19)
    
    let blockchain2 = Test.newEmulatorBlockchain()
    Test.assert(blockchain2.events().length == 19)
}

Acceptance Criteria

Either returns a newly-created backend implementation for every call of Test.newEmulatorBlockchain(), or maybe move towards making the Test.Blockchain a singleton. Creating many backend implementations should add some overhead when running tests, and right now, it is possible to call Reset() on a Test.Blockchain value. See discussion in #138.

Context

onflow/developer-grants#148

Problem

Running integration tests with code coverage enabled, does not collect/report any coverage metrics, due to the fact that the EmulatorBackend used the the test framework, has no CoverageReport object.

Steps to Reproduce

flow test --cover foo.cdc

Where foo.cdc uses something like:

pub var blockchain = Test.newEmulatorBlockchain()
var arrayUtils = Test.readFile("ArrayUtils.cdc")
var err = blockchain.deployContract(
    name: "ArrayUtils",
    code: arrayUtils,
    account: account,
    arguments: []
)

to deploy a contract on an account.

Acceptance Criteria

The above command should generate a valid coverage report and message, e.g:

Coverage: 55.5% of statements

for the contracts/scripts/transactions that were exercised during an integration test.

Context

This is part of an ongoing grant proposal: onflow/flow-emulator#333

Description

Authorized references (references with the auth keyword) allow downcasting, e.g. a restricted type to its unrestricted type, so should only be used in some specific cases. The subtype or unrestricted type could expose functionality that was not intended to be exposed.

Do not use authorized references when exposing functionality. For example, the fungible token standard provides an interface to get the balance of a vault, without exposing the withdrawal functionality.

https://developers.flow.com/cadence/anti-patterns#auth-references-and-capabilities-should-be-avoided

Acceptance criteria

• Linter warns the user when authorized references are defined for public capabilities or types
• Guidance to best practice is provided in the warning message

Issue to be solved

The generated comments for params and return values do not include the data type. It would be nice to have the data type printed for instance after the param name and after the Returns: statement

Issue To Be Solved

It would be really nice to have code-suggestions and auto-completions for more prominent use cases.

Suggest A Solution

• Can support auto-completions at different levels. e.g: Top-level, statement-level, expression-level, etc.
• Filter the results based on the context. e.g:
  • At the top-level, statements shouldn't be provided as suggestions.
  • Similarly, when the cursor is inside a contract, it should only provide suggestions of the constructs that are allowed within a contract.
• Support completions/suggestions when there are syntax errors.
• May require to introduce resilient parsing to support these.

Description

Access to an AuthAccount gives full access to the storage, keys, and contracts. Therefore using AuthAccount as a function parameter should be avoided

Acceptance criteria

• Liner emits a WARNING about using the AuthAccount in this way with a link guiding the user to the recommended practice instead

Tool: Cadence Language Server

Issue To Be Solved

There is two commands to get the parameters

• cadence.server.getContractInitializerParameters
• cadence.server.getEntryPointParameters

and one command to parse the parameters (for transactions and scripts)

• cadence.server.parseEntryPointArguments

Needed a parse command for contracts Initializer Parameters.

(Optional): Suggest A Solution

I see two options:

1. Create a fourth command for parsing contracts initialization parameters
2. Remove the contract specific command and use entry point commands for contracts as well as transactions and scripts.

(Optional): Context

Working on supporting contract initialization parameters in playground. The issue is an old one but keeps being requested by the community.

Playground Issues:
onflow/flow-playground#18
onflow/flow-playground#596

Issue To Be Solved

Code actions provide a convenient way for the users to fix issues in their code.

Suggest A Solution

• Compiler diagnostics (semantic errors) are generally used to figure out and provide code actions
• Some of the very useful code-actions are:
  • Create variables (when an undefined variable is referred)
  • Change access modifier
  • Add a type cast or conversion function call when there are incompatible types
  • Change variable/parameter/return/field type

Instructions

Problem

There seems to be some sort of issue resolving imported symbols. This appeared in onflow/vscode-cadence#312 when we updated vscode-langaugeclient from 8.0.2 to 8.1.0 - we have since downgraded to prevent this error message.

I think what happened was not that this actually created any new issue, but rather that vscode just was not displaying the error prior (see microsoft/vscode-languageserver-node@55eb8e3).

I did some logging in order to debug this. Below is the output of the textDocument/documentSymbol response for the FlowToken contract.

Notice how we have a symbol with no name (which is forbidden in LSP). This is the import symbol and we need to assign it a name.

Steps to Reproduce

Try to update vscode-languageclient in vscode-cadence and use the extension on a file with one or more imports. It will begin to throw the error.

Currently when the Test.assert function fails it doesn't provide much output besides the error: assertion failed which makes it hard to debug and know what the issue is.

I believe the output of assert function should be improved to be more like:

error: not equal, expected value: 1, actual value: 2, both values should be equal

Analyzers are amazing, it would be great if they also offer the fix.
For example: with LS and codelens, vs code can guide users to the fix within a click.

Something like: replace this document range with fix would be very valuable

onflow/cadence#1997 refactored the standard library values to be reusable.

Refactor the boilerplate code in the LS to use stdlib.DefaultScriptStandardLibraryValues instead.

Problem

Steps to Reproduce

Open a cadence script and attempt to use the built in getAuthAccount function.

Acceptance Criteria

No more red squiggly line.

Context

Using version v0.7.1 of the Cadence extension.

Issue To Be Solved

Currently, when writing integration tests with the Cadence testing framework, there is no way to send some amount of FLOW tokens to any newly-created account. Since FLOW is the native token of the blockchain, it is a must-have feature to be able to send such tokens to any account, and test relevant scenarios that require usage of FLOW tokens.

Suggest A Solution

Since the emulator is used under the hood for integration tests, one possible solution is to expose the service account to the testing framework. The service account contains a large amount of FLOW tokens, and developers will be able to write transactions and send any desired amount to newly-created accounts. The service account is necessary for signing such transactions. Helper methods might be defined as well, to avoid boilerplate code, and create new accounts with a predefined number of FLOW tokens.

Context

onflow/developer-grants#148

Feature Request

It is an anti-pattern to pass AuthAccounts to functions.

Detect and report parameters that have an AuthAccount type.

Problem

Attempting to compile the Language Server for WebAssembly fails, though it used to work:

Steps to Reproduce

$ cd languageserver
$ make wasm
GOARCH=wasm GOOS=js go build -o languageserver.wasm  ./cmd/languageserver
package github.com/onflow/cadence-tools/languageserver/cmd/languageserver
	imports github.com/onflow/cadence-tools/languageserver/server
	imports github.com/onflow/cadence-tools/lint
	imports github.com/onflow/flow-cli/pkg/flowkit/gateway
	imports github.com/onflow/flow-emulator
	imports github.com/onflow/flow-emulator/convert
	imports github.com/onflow/flow-emulator/convert/sdk
	imports github.com/onflow/flow-go/access
	imports github.com/onflow/flow-go/consensus/hotstuff
	imports github.com/onflow/flow-go/module
	imports github.com/onflow/flow-go/state/protocol
	imports github.com/onflow/flow-go/storage
	imports github.com/dgraph-io/badger/v2
	imports golang.org/x/sys/unix: build constraints exclude all Go files in /Users/bastian/go/pkg/mod/golang.org/x/[email protected]/unix
make: *** [wasm] Error 1

Acceptance Criteria

Compilation to WebAssembly should succeed like it used to.

make build results in:

GOARCH=wasm GOOS=js go build -o ./cmd/languageserver/languageserver.wasm ./cmd/languageserver
package github.com/onflow/cadence-tools/languageserver/cmd/languageserver
	imports github.com/onflow/cadence-tools/languageserver/server
	imports github.com/onflow/cadence-tools/lint
	imports github.com/onflow/flow-cli/pkg/flowkit/gateway
	imports github.com/onflow/flow-emulator
	imports github.com/onflow/flow-emulator/convert
	imports github.com/onflow/flow-emulator/convert/sdk
	imports github.com/onflow/flow-go/access
	imports github.com/onflow/flow-go/consensus/hotstuff/signature
	imports github.com/onflow/flow-go/storage
	imports github.com/dgraph-io/badger/v2
	imports golang.org/x/sys/unix: build constraints exclude all Go files in /Users/bluesign/go/pkg/mod/golang.org/x/[email protected]/unix
make: *** [build] Error 1

Issue To Be Solved

Currently, log statements are ignored when running tests.

Suggest A Solution

It would be great to get all log statements to get printed with some flag when running tests.

Context

https://github.com/orgs/onflow/discussions/1330

Importing test framework doesn't work. When doing import Test in order to use the new testing framework the checking fails.

VSCode extension is affected by this issue as it doesn't provide intellisense functionalities

Issue To Be Solved

Make it easier to read and learn Cadence code by allowing users to discover the meaning of source code elements, for example by providing hover documentation, for keywords, operators, and other syntax, in the language server.

As the language server is integrated into the Playground, this would provide way for a new user to understand tutorial code and existing contracts.

This idea is nicely described in https://oleb.net/2021/swift-language-reference/

Suggest A Solution

• Implement hover for keywords and tokens
  • transaction, prepare, and execute
  • pre, post, and result
  • AuthAccount
  • Resources:
    • resource keyword
    • Resource type annotation (@)
    • Move operator <-
    • create and destroy keyword
    • Shift statement (double move operator)
  • Restricted types
  • Access modifiers
  • Force-unwrap operator (!)
  • Optional chaining (?.)
  • ...

Issue To Be Solved

We need playground to support attachments, Currently language server says it does not support attachments.

Steps to Reproduce

Context

@MaxStalker created some great code snippets for VS Code. They're very convenient and we should provide them as a default in the language server, i.e. in the Visual Studio Code extension and the Playground, e.g.:

• New empty array literal
• Null-coalesce
• Fields + initializer
• Pre and post-condition
• If statement, if-else statement
• While loop
• For-in loop
• Optional binding, with and without else block

Definition of Done

• Port more code snippets as completion suggestions to the language server

Description

Linter should inform users that

1. Top level comments and comments for types, fields, events, and functions should use /// which will ensure it gets picked up by the Cadence doc autogeneration
2. Regular comments within functions should only use two slashes

Is your feature request related to a problem? Please describe.
Auto completion does not work for nested contexts, for example:

Describe the solution you'd like

Autocompletion should work and show properties and functions for account (acc)

Support the new import schema as part of the flowkit implementation onflow/flow-cli#711
LS should know how to resolve the imports using the existing flowkit implementation.

It would be nice if docgen could support scripts and
transactions so that you could document them with the examples.

In the case where the user already has an emulator instance running the LS should detect that and connect to it. We should also allow optional configuration option to pass in defining an emulator to use. It would contain a host address and a special value hosted forcing usage of self hosted emulator.

Currently, LS doesn't respond to changes in configuration, we need to implement the handler for workspace/didChangeConfiguration in LS and set updated values.

Support reloading flow.json configuration when changed as it affects the resolution of imported contracts.

Possible regression.

Importing with the new import syntax in scripts has some issues.

Here's a code example to reproduce:

import "Bar"

pub fun main(a: Int, b: Int): String {
    return Bar.X
}

The output looks like:

And the error is:
checking of imported "pub" program failed.

Currently, it's not possible to deploy contracts using LS with init arguments. We should implement pragma arguments that allow you to specify init values for deployment.

Currently, when we use access modifier access(account), vscode extention will treat all these contracts to be deployed in separate accounts and report error.

As the contracts should already be defined in flow.json, it should be possible to resolve whether the contracts are going to be under the same account?

Adding this feature can help a lot!
Thank you.

Problem
The language server should use flow.json configuration for accounts and expose more functionality such as deploying project contracts, etc.
Currently, when a user starts the LS client we create some accounts for the user, but those accounts are logically separate from those in flow.json which makes the whole thing quite confusing and it doesn't leverage the flowkit capability of using those generated accounts for sending transactions (we need to build and send those manually) but more importantly it doesn't provide more functionality of deploying contracts defined in flow.json.

Solution
Refactor the code so it adds the newly created accounts on top of the existing ones using the flowkit state API. We should also create the accounts defined in flow.json for the user during the setup.
The client can then detect if the flow.json is present and if so, allows the user to automatically deploy all the contracts configured. Down the line another feature could come where it would even migrate to a state.

Issue To Be Solved

Currently, there is no way to specify that a test is expected to fail/panic. For example:

flow test --cover MerkleProof_test.cdc
Running tests...

Test results: "MerkleProof_test.cdc"
- PASS: testVerifyValidProof
- FAIL: testVerifyInvalidProof
		Execution failed:
			error: panic: invalid proof
			  --> ./contracts/MerkleProof.cdc:12:16
			
- PASS: testVerifyProofWithLowerLeaf
- PASS: testVerifyWithHigherLeaf
Coverage: 100.0% of statements

For the testVerifyInvalidProof test case, the error: panic: invalid proof is the expected program behavior, however the test case is marked with a FAIL status.

Suggest A Solution

Add a built-in method/matcher, which wraps a method call execution and expects a panic with a certain error message.

Test.assertFailsWith(fun(): Void {
    merkleProof.verifyProof(
        proof: [proof.decodeHex()],
        root: root.decodeHex(),
        leaf: leaf.decodeHex(),
        hasherRawValue: 1
    )
}, "invalid proof")

Context

onflow/developer-grants#148

Feature Request

Programs should always use the most specific type possible, following the principle of least privilege.
Types should always be as specific (restrictive) as possible, especially for resource-types.
This can be accomplished by using restricted types and interfaces.

An analyzer could look at how a variable/parameter is used, then find the minimal set of interfaces that is needed to allow the uses, and finally recommend the minimal restricted type.

For example, if only the balance field of a FT Vault is read, the type could just be &Vault{Balance} instead of &Vault

The LS should be automatically built on the release and published as a new version on npm. This should work along with the playground frontend automated update.

Feature Request

Add a new analyzer which detects unused value-typed results. For example, the following code is likely a bug:

let string = "hello"
string.concat("world")

Suggested Solution

• Report ExpressionStatements of:
  • Function call with value-typed result
  • Literals (strings, boolean, array, dictionary, etc.)
  • Variable (no side-effect)

There's a windows specific issue when resolving string imports. The Sentry report:

Sentry Issue: LANGUAGE-SERVER-6V

runtime.errorString: runtime error: invalid memory address or nil pointer dereference
  File "/go/pkg/mod/github.com/onflow/cadence-tools/[email protected]/integration/resolvers.go", line 41, in (*resolvers).stringImport
  File "/go/pkg/mod/github.com/onflow/cadence-tools/[email protected]/server/server.go", line 2019, in (*Server).resolveImport
  File "/go/pkg/mod/github.com/onflow/cadence-tools/[email protected]/server/server.go", line 2870, in (*Server).handleImport
  File "/go/pkg/mod/github.com/onflow/[email protected]/runtime/sema/check_import_declaration.go", line 102, in (*Checker).importResolvedLocation
  File "/go/pkg/mod/github.com/onflow/[email protected]/runtime/sema/check_import_declaration.go", line 65, in (*Checker).declareImportDeclaration
...
(9 additional frame(s) were not displayed)

https://dapperlabs.sentry.io/issues/3901167811/events/latest/?project=6330569&query=is%3Aunresolved+level%3Afatal&referrer=latest-event&stream_index=0